From bdee0f82e4dac03f232d9c560a4ca1d9284152df Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Tue, 12 May 2015 17:49:29 -0700 Subject: [PATCH] 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 as the backing data when possible). --- .../Rewriters/LocalDeclarationRewriter.cs | 4 +- .../ExpressionCompiler/SymbolExtensions.cs | 6 +- .../Test/ExpressionCompiler/DynamicTests.cs | 2 +- .../DynamicFlagsCustomTypeInfoTests.cs | 4 +- .../ExpressionCompiler/CustomTypeInfo.cs | 11 +- .../DynamicFlagsCustomTypeInfo.cs | 118 ++++++++++-------- .../ExpressionCompiler/ExpressionCompiler.cs | 2 +- .../ResultProvider/Helpers/DynamicFlagsMap.cs | 2 +- .../ResultProvider/Helpers/DynamicHelpers.cs | 10 +- .../ResultProvider/ResultProviderTestBase.cs | 4 +- 10 files changed, 84 insertions(+), 79 deletions(-) diff --git a/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/Rewriters/LocalDeclarationRewriter.cs b/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/Rewriters/LocalDeclarationRewriter.cs index 505abe71ee3..14d644fb510 100644 --- a/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/Rewriters/LocalDeclarationRewriter.cs +++ b/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.GetInstance(numBytes); foreach (var b in bytes) { diff --git a/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/SymbolExtensions.cs b/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/SymbolExtensions.cs index 9cb93f02225..b324935c3ff 100644 --- a/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/SymbolExtensions.cs +++ b/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/SymbolExtensions.cs @@ -1,7 +1,7 @@ // 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 GetAllTypeParameters(this Me return builder.ToImmutableAndFree(); } - internal static byte[] GetCustomTypeInfoPayload(this MethodSymbol method) + internal static ReadOnlyCollection 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(); } } diff --git a/src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/DynamicTests.cs b/src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/DynamicTests.cs index a776d9d1595..d813a96e826 100644 --- a/src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/DynamicTests.cs +++ b/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()); } diff --git a/src/ExpressionEvaluator/CSharp/Test/ResultProvider/DynamicFlagsCustomTypeInfoTests.cs b/src/ExpressionEvaluator/CSharp/Test/ResultProvider/DynamicFlagsCustomTypeInfoTests.cs index b7958d65415..1579a66924b 100644 --- a/src/ExpressionEvaluator/CSharp/Test/ResultProvider/DynamicFlagsCustomTypeInfoTests.cs +++ b/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()); diff --git a/src/ExpressionEvaluator/Core/Source/ExpressionCompiler/CustomTypeInfo.cs b/src/ExpressionEvaluator/Core/Source/ExpressionCompiler/CustomTypeInfo.cs index 9f6ba82a520..53ea6689713 100644 --- a/src/ExpressionEvaluator/Core/Source/ExpressionCompiler/CustomTypeInfo.cs +++ b/src/ExpressionEvaluator/Core/Source/ExpressionCompiler/CustomTypeInfo.cs @@ -1,7 +1,6 @@ // 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 Payload; - public CustomTypeInfo(Guid payloadTypeId, byte[] payload) + public CustomTypeInfo(Guid payloadTypeId, ReadOnlyCollection 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(Payload)); + : DkmClrCustomTypeInfo.Create(PayloadTypeId, Payload); } } } diff --git a/src/ExpressionEvaluator/Core/Source/ExpressionCompiler/DynamicFlagsCustomTypeInfo.cs b/src/ExpressionEvaluator/Core/Source/ExpressionCompiler/DynamicFlagsCustomTypeInfo.cs index e2639695e73..e7ef5a1c392 100644 --- a/src/ExpressionEvaluator/Core/Source/ExpressionCompiler/DynamicFlagsCustomTypeInfo.cs +++ b/src/ExpressionEvaluator/Core/Source/ExpressionCompiler/DynamicFlagsCustomTypeInfo.cs @@ -1,7 +1,6 @@ // 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 /// Internal for testing. internal static readonly Guid PayloadTypeId = new Guid("826D6EC1-DC4B-46AF-BE05-CD3F1A1FD4AC"); - private readonly BitArray _bits; + private readonly ReadOnlyCollection _bytes; - public DynamicFlagsCustomTypeInfo(BitArray bits) + private DynamicFlagsCustomTypeInfo(ReadOnlyCollection bytes) + { + _bytes = bytes; + } + + public DynamicFlagsCustomTypeInfo(Guid payloadTypeId, ReadOnlyCollection 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.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(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; } } /// - /// 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 . /// It may have more (for padding) or fewer (for compactness) falses. /// It is, however, guaranteed to include the last true. /// internal void CopyTo(ArrayBuilder 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(payload)); - } + internal ReadOnlyCollection 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(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; } diff --git a/src/ExpressionEvaluator/Core/Source/ExpressionCompiler/ExpressionCompiler.cs b/src/ExpressionEvaluator/Core/Source/ExpressionCompiler/ExpressionCompiler.cs index b81b9961039..52136a2c0fd 100644 --- a/src/ExpressionEvaluator/Core/Source/ExpressionCompiler/ExpressionCompiler.cs +++ b/src/ExpressionEvaluator/Core/Source/ExpressionCompiler/ExpressionCompiler.cs @@ -87,7 +87,7 @@ private static ImmutableArray GetAliases(DkmClrRuntimeInstance runtimeIns dkmAlias.Type, new CustomTypeInfo( dkmAlias.CustomTypeInfoPayloadTypeId, - dkmAlias.CustomTypeInfoPayload.ToArray()))); + dkmAlias.CustomTypeInfoPayload))); } return builder.ToImmutableAndFree(); } diff --git a/src/ExpressionEvaluator/Core/Source/ResultProvider/Helpers/DynamicFlagsMap.cs b/src/ExpressionEvaluator/Core/Source/ResultProvider/Helpers/DynamicFlagsMap.cs index 34678887e6e..6992257afb7 100644 --- a/src/ExpressionEvaluator/Core/Source/ResultProvider/Helpers/DynamicFlagsMap.cs +++ b/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 builder) diff --git a/src/ExpressionEvaluator/Core/Source/ResultProvider/Helpers/DynamicHelpers.cs b/src/ExpressionEvaluator/Core/Source/ResultProvider/Helpers/DynamicHelpers.cs index 53bf3feda41..f869be2750a 100644 --- a/src/ExpressionEvaluator/Core/Source/ResultProvider/Helpers/DynamicHelpers.cs +++ b/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 attributes) { @@ -31,12 +31,12 @@ public static DynamicFlagsCustomTypeInfo GetDynamicFlags(this IList)arguments[0].Value; var numFlags = collection.Count; - var array = new BitArray(numFlags); - for (int i = 0; i < numFlags; i++) + var builder = ArrayBuilder.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()); } } } diff --git a/src/ExpressionEvaluator/Core/Test/ResultProvider/ResultProviderTestBase.cs b/src/ExpressionEvaluator/Core/Test/ResultProvider/ResultProviderTestBase.cs index 5f13bca4bce..01ec39c8863 100644 --- a/src/ExpressionEvaluator/Core/Test/ResultProvider/ResultProviderTestBase.cs +++ b/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.GetHashCode(DkmCustomUIVisualiz internal static DynamicFlagsCustomTypeInfo MakeDynamicFlagsCustomTypeInfo(params bool[] dynamicFlags) { - return new DynamicFlagsCustomTypeInfo(dynamicFlags == null ? null : new BitArray(dynamicFlags)); + return new DynamicFlagsCustomTypeInfo(dynamicFlags); } } } -- GitLab