Simplify DynamicFlagsCustomTypeInfo

The BitArray approach wasn't meaningfully reducing complexity, so we'll
revert to an earlier approach that reduces allocations (by reusing an
existing ReadOnlyCollection<byte> as the backing data when possible).

src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/Rewriters/LocalDeclarationRewriter.cs

@@ -114,7 +114,7 @@ private static BoundExpression GetCustomTypeInfoPayload(LocalSymbol local, CShar
                 compilation.GetSpecialType(SpecialType.System_Byte));
 
             var flags = CSharpCompilation.DynamicTransformsEncoder.Encode(local.Type, customModifiersCount: 0, refKind: RefKind.None).ToArray();
-            var bytes = new DynamicFlagsCustomTypeInfo(new BitArray(flags)).GetCustomTypeInfoPayload();
+            var bytes = new DynamicFlagsCustomTypeInfo(flags).GetCustomTypeInfoPayload();
             hasCustomTypeInfoPayload = bytes != null;
             if (!hasCustomTypeInfoPayload)
             {
@@ -124,7 +124,7 @@ private static BoundExpression GetCustomTypeInfoPayload(LocalSymbol local, CShar
             var byteType = byteArrayType.ElementType;
             var intType = compilation.GetSpecialType(SpecialType.System_Int32);
 
-            var numBytes = bytes.Length;
+            var numBytes = bytes.Count;
             var initializerExprs = ArrayBuilder<BoundExpression>.GetInstance(numBytes);
             foreach (var b in bytes)
             {

src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/SymbolExtensions.cs

 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
-using System.Collections;
 using System.Collections.Immutable;
+using System.Collections.ObjectModel;
 using System.Linq;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
 using Microsoft.CodeAnalysis.ExpressionEvaluator;
@@ -18,10 +18,10 @@ internal static ImmutableArray<TypeParameterSymbol> GetAllTypeParameters(this Me
             return builder.ToImmutableAndFree();
         }
 
-        internal static byte[] GetCustomTypeInfoPayload(this MethodSymbol method)
+        internal static ReadOnlyCollection<byte> GetCustomTypeInfoPayload(this MethodSymbol method)
         {
             bool[] dynamicFlags = CSharpCompilation.DynamicTransformsEncoder.Encode(method.ReturnType, method.ReturnTypeCustomModifiers.Length, RefKind.None).ToArray();
-            var dynamicFlagsInfo = new DynamicFlagsCustomTypeInfo(new BitArray(dynamicFlags));
+            var dynamicFlagsInfo = new DynamicFlagsCustomTypeInfo(dynamicFlags);
             return dynamicFlagsInfo.GetCustomTypeInfoPayload();
         }
     }

src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/DynamicTests.cs

@@ -546,7 +546,7 @@ .maxstack  1
 
         private static CustomTypeInfo MakeCustomTypeInfo(params bool[] flags)
         {
-            var dynamicFlagsInfo = new DynamicFlagsCustomTypeInfo(new BitArray(flags));
+            var dynamicFlagsInfo = new DynamicFlagsCustomTypeInfo(flags);
             return new CustomTypeInfo(DynamicFlagsCustomTypeInfo.PayloadTypeId, dynamicFlagsInfo.GetCustomTypeInfoPayload());
         }
 

src/ExpressionEvaluator/CSharp/Test/ResultProvider/DynamicFlagsCustomTypeInfoTests.cs

@@ -14,7 +14,7 @@ namespace Microsoft.CodeAnalysis.CSharp.UnitTests
     public class DynamicFlagsCustomTypeInfoTests : CSharpResultProviderTestBase
     {
         [Fact]
-        public void BitArrayConstructor()
+        public void BoolArrayConstructor()
         {
             ValidateBytes(MakeDynamicFlagsCustomTypeInfo(new bool[0]));
 
@@ -102,7 +102,7 @@ public void Any()
         [Fact]
         public void SkipOne()
         {
-            var dynamicFlagsCustomTypeInfo = new DynamicFlagsCustomTypeInfo((BitArray)null);
+            var dynamicFlagsCustomTypeInfo = new DynamicFlagsCustomTypeInfo(DynamicFlagsCustomTypeInfo.PayloadTypeId, null);
 
             ValidateBytes(dynamicFlagsCustomTypeInfo.SkipOne());
 

src/ExpressionEvaluator/Core/Source/ExpressionCompiler/CustomTypeInfo.cs

 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
 using System;
-using System.Collections;
 using System.Collections.ObjectModel;
 using Microsoft.VisualStudio.Debugger.Evaluation.ClrCompilation;
 
@@ -15,9 +14,9 @@ namespace Microsoft.CodeAnalysis.ExpressionEvaluator
     internal struct CustomTypeInfo
     {
         public Guid PayloadTypeId;
-        public byte[] Payload;
+        public ReadOnlyCollection<byte> Payload;
 
-        public CustomTypeInfo(Guid payloadTypeId, byte[] payload)
+        public CustomTypeInfo(Guid payloadTypeId, ReadOnlyCollection<byte> payload)
         {
             this.PayloadTypeId = payloadTypeId;
             this.Payload = payload;
@@ -25,16 +24,14 @@ public CustomTypeInfo(Guid payloadTypeId, byte[] payload)
 
         public DynamicFlagsCustomTypeInfo ToDynamicFlagsCustomTypeInfo()
         {
-            return PayloadTypeId == DynamicFlagsCustomTypeInfo.PayloadTypeId
-                ? new DynamicFlagsCustomTypeInfo(new BitArray(Payload))
-                : default(DynamicFlagsCustomTypeInfo);
+            return new DynamicFlagsCustomTypeInfo(PayloadTypeId, Payload);
         }
 
         public DkmClrCustomTypeInfo ToDkmClrCustomTypeInfo()
         {
             return Payload == null
                 ? null
-                : DkmClrCustomTypeInfo.Create(PayloadTypeId, new ReadOnlyCollection<byte>(Payload));
+                : DkmClrCustomTypeInfo.Create(PayloadTypeId, Payload);
         }
     }
 }

src/ExpressionEvaluator/Core/Source/ExpressionCompiler/DynamicFlagsCustomTypeInfo.cs

 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
 using System;
-using System.Collections;
 using System.Collections.ObjectModel;
 using System.Diagnostics;
 using Microsoft.VisualStudio.Debugger.Evaluation.ClrCompilation;
@@ -13,25 +12,55 @@ internal struct DynamicFlagsCustomTypeInfo
         /// <remarks>Internal for testing.</remarks>
         internal static readonly Guid PayloadTypeId = new Guid("826D6EC1-DC4B-46AF-BE05-CD3F1A1FD4AC");
 
-        private readonly BitArray _bits;
+        private readonly ReadOnlyCollection<byte> _bytes;
 
-        public DynamicFlagsCustomTypeInfo(BitArray bits)
+        private DynamicFlagsCustomTypeInfo(ReadOnlyCollection<byte> bytes)
+        {
+            _bytes = bytes;
+        }
+
+        public DynamicFlagsCustomTypeInfo(Guid payloadTypeId, ReadOnlyCollection<byte> payload)
+            : this(payloadTypeId == PayloadTypeId ? payload : null)
         {
-            _bits = bits;
         }
 
         public DynamicFlagsCustomTypeInfo(DkmClrCustomTypeInfo typeInfo)
+            : this(typeInfo != null && typeInfo.PayloadTypeId == PayloadTypeId ? typeInfo.Payload : null)
+        {
+        }
+
+        internal DynamicFlagsCustomTypeInfo(bool[] dynamicFlags)
         {
-            if (typeInfo == null || typeInfo.PayloadTypeId != PayloadTypeId)
+            if (dynamicFlags == null || dynamicFlags.Length == 0)
             {
-                _bits = null;
+                _bytes = null;
+                return;
             }
-            else
+
+            int numFlags = dynamicFlags.Length;
+            int numBytes = ((numFlags - 1) / 8) + 1;
+            byte[] bytes = new byte[numBytes];
+            for (int b = 0; b < numBytes; b++)
             {
-                var builder = ArrayBuilder<byte>.GetInstance();
-                builder.AddRange(typeInfo.Payload);
-                _bits = new BitArray(builder.ToArrayAndFree());
+                for (int i = 0; i < 8; i++)
+                {
+                    var f = b * 8 + i;
+                    if (f >= numFlags)
+                    {
+                        Debug.Assert(f == numFlags);
+                        goto ALL_FLAGS_READ;
+                    }
+
+                    if (dynamicFlags[f])
+                    {
+                        bytes[b] |= (byte)(1 << i);
+                    }
+                }
             }
+
+            ALL_FLAGS_READ:
+
+            _bytes = new ReadOnlyCollection<byte>(bytes);
         }
 
         public bool this[int i]
@@ -39,91 +68,70 @@ public DynamicFlagsCustomTypeInfo(DkmClrCustomTypeInfo typeInfo)
             get
             {
                 Debug.Assert(i >= 0);
-                return _bits != null &&
-                    i < _bits.Length &&
-                    _bits[i];
+                var b = i / 8;
+                return _bytes != null &&
+                    b < _bytes.Count &&
+                    (_bytes[b] & (1 << (i % 8))) != 0;
             }
         }
 
         /// <remarks>
-        /// Not guarantee to add the same number of flags as would
+        /// Not guaranteed to add the same number of flags as would
         /// appear in a <see cref="System.Runtime.CompilerServices.DynamicAttribute"/>.
         /// It may have more (for padding) or fewer (for compactness) falses.
         /// It is, however, guaranteed to include the last true.
         /// </remarks>
         internal void CopyTo(ArrayBuilder<bool> builder)
         {
-            if (_bits == null)
+            if (_bytes == null)
             {
                 return;
             }
 
-            for (int b = 0; b < _bits.Length; b++)
-            {
-                builder.Add(_bits[b]);
-            }
-        }
-
-        internal byte[] GetCustomTypeInfoPayload()
-        {
-            if (!Any())
-            {
-                return null;
-            }
-
-            var numBits = _bits.Length;
-            var numBytes = (numBits + 7) / 8;
-            var bytes = new byte[numBytes];
-
-            // Unfortunately, BitArray.CopyTo is not portable.
-            for (int b = 0; b < numBytes; b++)
+            foreach (byte b in _bytes)
             {
                 for (int i = 0; i < 8; i++)
                 {
-                    var index = 8 * b + i;
-                    if (index < numBits && _bits[index])
-                    {
-                        bytes[b] |= (byte)(1 << i);
-                    }
+                    builder.Add((b & (1 << i)) != 0);
                 }
             }
-
-            return bytes;
         }
 
-        public DkmClrCustomTypeInfo GetCustomTypeInfo()
-        {
-            var payload = GetCustomTypeInfoPayload();
-            return payload == null ? null : DkmClrCustomTypeInfo.Create(PayloadTypeId, new ReadOnlyCollection<byte>(payload));
-        }
+        internal ReadOnlyCollection<byte> GetCustomTypeInfoPayload() => Any() ? _bytes : null;
+
+        public DkmClrCustomTypeInfo GetCustomTypeInfo() => Any() ? DkmClrCustomTypeInfo.Create(PayloadTypeId, _bytes) : null;
 
         public DynamicFlagsCustomTypeInfo SkipOne()
         {
-            if (_bits == null)
+            if (_bytes == null)
             {
                 return this;
             }
 
-            var numBits = _bits.Length;
-            var newBits = new BitArray(numBits - 1);
-            for (int b = 0; b < numBits - 1; b++)
+            var numBytes = _bytes.Count;
+            var newBytes = new byte[numBytes]; // CONSIDER: In some cases, we could shrink the array.
+            for (int b = 0; b < numBytes; b++)
             {
-                newBits[b] = _bits[b + 1];
+                newBytes[b] = (byte)(_bytes[b] >> 1);
+                if (b + 1 < numBytes && (_bytes[b + 1] & 1) != 0)
+                {
+                    newBytes[b] |= 1 << 7;
+                }
             }
 
-            return new DynamicFlagsCustomTypeInfo(newBits);
+            return new DynamicFlagsCustomTypeInfo(new ReadOnlyCollection<byte>(newBytes));
         }
 
         public bool Any()
         {
-            if (_bits == null)
+            if (_bytes == null)
             {
                 return false;
             }
 
-            for (int b = 0; b < _bits.Length; b++)
+            for (int b = 0; b < _bytes.Count; b++)
             {
-                if (_bits[b])
+                if (_bytes[b] != 0)
                 {
                     return true;
                 }

src/ExpressionEvaluator/Core/Source/ExpressionCompiler/ExpressionCompiler.cs

@@ -87,7 +87,7 @@ private static ImmutableArray<Alias> GetAliases(DkmClrRuntimeInstance runtimeIns
                     dkmAlias.Type,
                     new CustomTypeInfo(
                         dkmAlias.CustomTypeInfoPayloadTypeId,
-                        dkmAlias.CustomTypeInfoPayload.ToArray())));
+                        dkmAlias.CustomTypeInfoPayload)));
             }
             return builder.ToImmutableAndFree();
         }

src/ExpressionEvaluator/Core/Source/ResultProvider/Helpers/DynamicFlagsMap.cs

@@ -97,7 +97,7 @@ internal DynamicFlagsCustomTypeInfo SubstituteDynamicFlags(Type type, DynamicFla
                 f++;
             }
 
-            return new DynamicFlagsCustomTypeInfo(new BitArray(substitutedFlags.ToArrayAndFree()));
+            return new DynamicFlagsCustomTypeInfo(substitutedFlags.ToArrayAndFree());
         }
 
         private void AppendFlagsFor(Type type, ArrayBuilder<bool> builder)

src/ExpressionEvaluator/Core/Source/ResultProvider/Helpers/DynamicHelpers.cs

@@ -9,7 +9,7 @@ namespace Microsoft.CodeAnalysis.ExpressionEvaluator
 {
     internal static class DynamicHelpers
     {
-        private static readonly BitArray TrueArray = new BitArray(new[] { true });
+        private static readonly bool[] TrueArray = new[] { true };
 
         public static DynamicFlagsCustomTypeInfo GetDynamicFlags(this IList<CustomAttributeData> attributes)
         {
@@ -31,12 +31,12 @@ public static DynamicFlagsCustomTypeInfo GetDynamicFlags(this IList<CustomAttrib
                             // if ArgumentType indicates an array, then Value will actually be a ReadOnlyCollection.
                             var collection = (ReadOnlyCollection<CustomAttributeTypedArgument>)arguments[0].Value;
                             var numFlags = collection.Count;
-                            var array = new BitArray(numFlags);
-                            for (int i = 0; i < numFlags; i++)
+                            var builder = ArrayBuilder<bool>.GetInstance(numFlags);
+                            foreach (var typedArg in collection)
                             {
-                                array[i] = (bool)collection[i].Value;
+                                builder.Add((bool)typedArg.Value);
                             }
-                            return new DynamicFlagsCustomTypeInfo(array);
+                            return new DynamicFlagsCustomTypeInfo(builder.ToArrayAndFree());
                         }
                     }
                 }

src/ExpressionEvaluator/Core/Test/ResultProvider/ResultProviderTestBase.cs

@@ -162,7 +162,7 @@ internal DkmEvaluationResult FormatResult(string name, string fullName, DkmClrVa
                 value,
                 workList,
                 declaredType: declaredType ?? value.Type,
-                customTypeInfo: new DynamicFlagsCustomTypeInfo(declaredTypeInfo == null ? null : new BitArray(declaredTypeInfo)).GetCustomTypeInfo(),
+                customTypeInfo: new DynamicFlagsCustomTypeInfo(declaredTypeInfo).GetCustomTypeInfo(),
                 inspectionContext: inspectionContext ?? DefaultInspectionContext,
                 formatSpecifiers: Formatter.NoFormatSpecifiers,
                 resultName: name,
@@ -486,7 +486,7 @@ int IEqualityComparer<DkmCustomUIVisualizerInfo>.GetHashCode(DkmCustomUIVisualiz
 
         internal static DynamicFlagsCustomTypeInfo MakeDynamicFlagsCustomTypeInfo(params bool[] dynamicFlags)
         {
-            return new DynamicFlagsCustomTypeInfo(dynamicFlags == null ? null : new BitArray(dynamicFlags));
+            return new DynamicFlagsCustomTypeInfo(dynamicFlags);
         }
     }
 }