From 3d44ea6a099e2f8042ece5303467bef91f10b512 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Mon, 23 Jan 2023 20:57:05 +0200 Subject: [PATCH 1/7] Add `BlobDictionary`. --- .../src/System.Reflection.Metadata.csproj | 1 + .../Internal/Utilities/BlobDictionary.cs | 99 +++++++++++++++++++ 2 files changed, 100 insertions(+) create mode 100644 src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/BlobDictionary.cs diff --git a/src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj b/src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj index 04cbedc65b3c70..a33b2fff9ace9c 100644 --- a/src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj +++ b/src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj @@ -23,6 +23,7 @@ The System.Reflection.Metadata library is built-in as part of the shared framewo + diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/BlobDictionary.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/BlobDictionary.cs new file mode 100644 index 00000000000000..968279e053c3da --- /dev/null +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/BlobDictionary.cs @@ -0,0 +1,99 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Diagnostics; +#if NET +using System.Runtime.InteropServices; +#endif + +namespace System.Reflection.Internal +{ + [DebuggerDisplay("Count = {Count}")] + internal readonly struct BlobDictionary + { + private readonly Dictionary, TValue>> _dictionary; + + // A simple LCG. Constants taken from + // https://github.com/imneme/pcg-c/blob/83252d9c23df9c82ecb42210afed61a7b42402d7/include/pcg_variants.h#L276-L284 + private static int GetNextDictionaryKey(int dictionaryKey) => + (int)((uint)dictionaryKey * 747796405 + 2891336453); + +#if NET + private ref KeyValuePair, TValue> GetValueRefOrAddDefault(ReadOnlySpan key, out bool exists) + { + int dictionaryKey = Hash.GetFNVHashCode(key); + while (true) + { + ref var entry = ref CollectionsMarshal.GetValueRefOrAddDefault(_dictionary, dictionaryKey, out exists); + if (!exists || entry.Key.AsSpan().SequenceEqual(key)) + { + return ref entry; + } + dictionaryKey = GetNextDictionaryKey(dictionaryKey); + } + } + + private TValue GetOrAdd(ReadOnlySpan key, ImmutableArray immutableKey, TValue value, out bool exists) + { + ref var entry = ref GetValueRefOrAddDefault(key, out exists); + if (exists) + { + return entry.Value; + } + + if (immutableKey.IsDefault) + { + immutableKey = key.ToImmutableArray(); + } + entry = new(immutableKey, value); + return value; + } +#else + private TValue GetOrAdd(ReadOnlySpan key, ImmutableArray immutableKey, TValue value, out bool exists) + { + int dictionarykey = Hash.GetFNVHashCode(key); + KeyValuePair, TValue> entry; + while (true) + { + if (!(exists = _dictionary.TryGetValue(dictionarykey, out entry)) + || entry.Key.AsSpan().SequenceEqual(key)) + { + break; + } + dictionarykey = GetNextDictionaryKey(dictionarykey); + } + + if (exists) + { + return entry.Value; + } + + if (immutableKey.IsDefault) + { + immutableKey = key.ToImmutableArray(); + } + _dictionary.Add(dictionarykey, new(immutableKey, value)); + return value; + } +#endif + + public BlobDictionary(int capacity = 0) + { + _dictionary = new(capacity); + } + + public int Count => _dictionary.Count; + + public IEnumerator, TValue>>> GetEnumerator() => + _dictionary.GetEnumerator(); + + public TValue GetOrAdd(ReadOnlySpan key, TValue value, out bool exists) => + GetOrAdd(key, default, value, out exists); + + // If we are given an immutable array, do not allocate a new one. + public TValue GetOrAdd(ImmutableArray key, TValue value, out bool exists) => + GetOrAdd(key.AsSpan(), key, value, out exists); + } +} From 0cf8b9c28df13e8c65491fe548b121b826b16555 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Mon, 23 Jan 2023 22:06:15 +0200 Subject: [PATCH 2/7] Use `BlobDictionary` in `MetadataReader` and optimize the simple cases. --- .../Metadata/Ecma335/MetadataBuilder.Heaps.cs | 31 ++++++++++++------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataBuilder.Heaps.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataBuilder.Heaps.cs index a58bf9f1f7883d..fbb65a50f49d1f 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataBuilder.Heaps.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataBuilder.Heaps.cs @@ -44,7 +44,7 @@ internal void SetCapacity(int capacity) private int _stringHeapCapacity = 4 * 1024; // #Blob heap - private readonly Dictionary, BlobHandle> _blobs = new Dictionary, BlobHandle>(1024, ByteSequenceComparer.Instance); + private readonly BlobDictionary _blobs = new BlobDictionary(1024); private readonly int _blobHeapStartOffset; private int _blobHeapSize; @@ -118,7 +118,7 @@ public MetadataBuilder( // beginning of the delta blob. _userStringBuilder.WriteByte(0); - _blobs.Add(ImmutableArray.Empty, default(BlobHandle)); + _blobs.GetOrAdd(ImmutableArray.Empty, default, out _); _blobHeapSize = 1; // When EnC delta is applied #US, #String and #Blob heaps are appended. @@ -210,8 +210,19 @@ public BlobHandle GetOrAddBlob(byte[] value) Throw.ArgumentNull(nameof(value)); } - // TODO: avoid making a copy if the blob exists in the index - return GetOrAddBlob(ImmutableArray.Create(value)); + return GetOrAddBlob(new ReadOnlySpan(value)); + } + + private BlobHandle GetOrAddBlob(ReadOnlySpan value) + { + BlobHandle nextHandle = BlobHandle.FromOffset(_blobHeapStartOffset + _blobHeapSize); + BlobHandle handle = _blobs.GetOrAdd(value, nextHandle, out bool exists); + if (!exists) + { + _blobHeapSize += BlobWriterImpl.GetCompressedIntegerSize(value.Length) + value.Length; + } + + return handle; } /// @@ -227,12 +238,10 @@ public BlobHandle GetOrAddBlob(ImmutableArray value) Throw.ArgumentNull(nameof(value)); } - BlobHandle handle; - if (!_blobs.TryGetValue(value, out handle)) + BlobHandle nextHandle = BlobHandle.FromOffset(_blobHeapStartOffset + _blobHeapSize); + BlobHandle handle = _blobs.GetOrAdd(value, nextHandle, out bool exists); + if (!exists) { - handle = BlobHandle.FromOffset(_blobHeapStartOffset + _blobHeapSize); - _blobs.Add(value, handle); - _blobHeapSize += BlobWriterImpl.GetCompressedIntegerSize(value.Length) + value.Length; } @@ -611,8 +620,8 @@ private void WriteAlignedBlobHeap(BlobBuilder builder) int startOffset = _blobHeapStartOffset; foreach (var entry in _blobs) { - int heapOffset = entry.Value.GetHeapOffset(); - var blob = entry.Key; + int heapOffset = entry.Value.Value.GetHeapOffset(); + var blob = entry.Value.Key; writer.Offset = (heapOffset == 0) ? 0 : heapOffset - startOffset; writer.WriteCompressedInteger(blob.Length); From a7b99d49289a1cf79af4c7551c51c852ebf215a3 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Mon, 23 Jan 2023 22:59:20 +0200 Subject: [PATCH 3/7] Optimize adding blobs from UTF-16 strings and from blob builders. --- .../System/Reflection/Metadata/BlobBuilder.cs | 32 +++++++++++++++++-- .../Metadata/Ecma335/MetadataBuilder.Heaps.cs | 20 +++++++++--- 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/BlobBuilder.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/BlobBuilder.cs index 8debffc0e94761..b99c45dc73f7d6 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/BlobBuilder.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/BlobBuilder.cs @@ -50,6 +50,7 @@ public partial class BlobBuilder private bool IsHead => (_length & IsFrozenMask) == 0; private int Length => (int)(_length & ~IsFrozenMask); private uint FrozenLength => _length | IsFrozenMask; + private Span Span => _buffer.AsSpan(0, Length); public BlobBuilder(int capacity = DefaultChunkSize) { @@ -318,6 +319,33 @@ public ImmutableArray ToImmutableArray(int start, int byteCount) return ImmutableByteArrayInterop.DangerousCreateFromUnderlyingArray(ref array); } + internal bool TryWriteTo(Span scratchBuffer, out ReadOnlySpan buffer) + { + if (_nextOrPrevious == this) + { + // If the blob builder has one chunk, we can just return it and avoid copies. + buffer = Span; + return true; + } + + int count = Count; + if (scratchBuffer.Length <= count) + { + int i = 0; + foreach (var chunk in GetChunks()) + { + chunk.Span.CopyTo(scratchBuffer.Slice(i)); + i += chunk.Length; + } + + buffer = scratchBuffer.Slice(0, count); + return true; + } + + buffer = default; + return false; + } + /// is null. /// Content is not available, the builder has been linked with another one. public void WriteContentTo(Stream destination) @@ -344,7 +372,7 @@ public void WriteContentTo(ref BlobWriter destination) foreach (var chunk in GetChunks()) { - destination.WriteBytes(chunk._buffer.AsSpan(0, chunk.Length)); + destination.WriteBytes(chunk.Span); } } @@ -359,7 +387,7 @@ public void WriteContentTo(BlobBuilder destination) foreach (var chunk in GetChunks()) { - destination.WriteBytes(chunk._buffer.AsSpan(0, chunk.Length)); + destination.WriteBytes(chunk.Span); } } diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataBuilder.Heaps.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataBuilder.Heaps.cs index fbb65a50f49d1f..65df9df609e7a3 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataBuilder.Heaps.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataBuilder.Heaps.cs @@ -3,10 +3,8 @@ using System.Collections.Generic; using System.Collections.Immutable; -using System.Diagnostics; using System.Reflection.Internal; -using System.Runtime.CompilerServices; -using System.Text; +using System.Runtime.InteropServices; namespace System.Reflection.Metadata.Ecma335 { @@ -193,7 +191,11 @@ public BlobHandle GetOrAddBlob(BlobBuilder value) Throw.ArgumentNull(nameof(value)); } - // TODO: avoid making a copy if the blob exists in the index + if (value.TryWriteTo(stackalloc byte[256], out ReadOnlySpan buffer)) + { + return GetOrAddBlob(buffer); + } + return GetOrAddBlob(value.ToImmutableArray()); } @@ -276,6 +278,16 @@ public unsafe BlobHandle GetOrAddConstantBlob(object? value) /// is null. public BlobHandle GetOrAddBlobUTF16(string value) { + if (value is null) + { + Throw.ArgumentNull(nameof(value)); + } + + if (BitConverter.IsLittleEndian) + { + return GetOrAddBlob(MemoryMarshal.AsBytes(value.AsSpan())); + } + var builder = PooledBlobBuilder.GetInstance(); builder.WriteUTF16(value); var handle = GetOrAddBlob(builder); From 1011fb3f8d0dd5076e267f33b5cde137fb1ae3ac Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Mon, 23 Jan 2023 23:08:41 +0200 Subject: [PATCH 4/7] Delete `ByteSequenceComparer`. --- .../src/System.Reflection.Metadata.csproj | 1 - .../Utilities/ByteSequenceComparer.cs | 68 ------------------- .../System/Reflection/Metadata/BlobBuilder.cs | 2 +- .../System/Reflection/Metadata/BlobWriter.cs | 2 +- .../tests/Metadata/BlobTests.cs | 2 +- 5 files changed, 3 insertions(+), 72 deletions(-) delete mode 100644 src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/ByteSequenceComparer.cs diff --git a/src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj b/src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj index a33b2fff9ace9c..6b2539dd8f85e3 100644 --- a/src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj +++ b/src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj @@ -46,7 +46,6 @@ The System.Reflection.Metadata library is built-in as part of the shared framewo - diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/ByteSequenceComparer.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/ByteSequenceComparer.cs deleted file mode 100644 index 2ddd1806da2f3f..00000000000000 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/ByteSequenceComparer.cs +++ /dev/null @@ -1,68 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Collections.Generic; -using System.Collections.Immutable; -using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; - -namespace System.Reflection.Internal -{ - internal sealed class ByteSequenceComparer : IEqualityComparer, IEqualityComparer> - { - internal static readonly ByteSequenceComparer Instance = new ByteSequenceComparer(); - - private ByteSequenceComparer() - { - } - - internal static bool Equals(ImmutableArray x, ImmutableArray y) - { - return x.AsSpan().SequenceEqual(y.AsSpan()); - } - - internal static bool Equals(byte[] left, int leftStart, byte[] right, int rightStart, int length) - { - return left.AsSpan(leftStart, length).SequenceEqual(right.AsSpan(rightStart, length)); - } - - internal static bool Equals(byte[]? left, byte[]? right) - { - return left.AsSpan().SequenceEqual(right.AsSpan()); - } - - // Both hash computations below use the FNV-1a algorithm (http://en.wikipedia.org/wiki/Fowler%E2%80%93Noll%E2%80%93Vo_hash_function). - - internal static int GetHashCode(byte[] x) - { - Debug.Assert(x != null); - return Hash.GetFNVHashCode(x); - } - - internal static int GetHashCode(ImmutableArray x) - { - Debug.Assert(!x.IsDefault); - return Hash.GetFNVHashCode(x.AsSpan()); - } - - bool IEqualityComparer.Equals(byte[]? x, byte[]? y) - { - return Equals(x, y); - } - - int IEqualityComparer.GetHashCode(byte[] x) - { - return GetHashCode(x); - } - - bool IEqualityComparer>.Equals(ImmutableArray x, ImmutableArray y) - { - return Equals(x, y); - } - - int IEqualityComparer>.GetHashCode(ImmutableArray x) - { - return GetHashCode(x); - } - } -} diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/BlobBuilder.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/BlobBuilder.cs index b99c45dc73f7d6..4ad7d70e4b3053 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/BlobBuilder.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/BlobBuilder.cs @@ -235,7 +235,7 @@ public bool ContentEquals(BlobBuilder other) var right = rightEnumerator.Current; int minLength = Math.Min(left.Length - leftStart, right.Length - rightStart); - if (!ByteSequenceComparer.Equals(left._buffer, leftStart, right._buffer, rightStart, minLength)) + if (!left._buffer.AsSpan(leftStart, minLength).SequenceEqual(right._buffer.AsSpan(rightStart, minLength))) { return false; } diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/BlobWriter.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/BlobWriter.cs index fdd73e33ba289a..a9bd1288111556 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/BlobWriter.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/BlobWriter.cs @@ -54,7 +54,7 @@ public BlobWriter(byte[] buffer, int start, int count) /// public bool ContentEquals(BlobWriter other) { - return Length == other.Length && ByteSequenceComparer.Equals(_buffer, _start, other._buffer, other._start, Length); + return Length == other.Length && _buffer.AsSpan(_start, Length).SequenceEqual(other._buffer.AsSpan(other._start, other.Length)); } public int Offset diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/BlobTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/BlobTests.cs index cefd6e109d4e3b..6c05f36d046c9f 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/BlobTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/BlobTests.cs @@ -61,7 +61,7 @@ private void TestContentEquals(byte[] left, byte[] right) var builder2 = new BlobBuilder(0); builder2.WriteBytes(right); - bool expected = ByteSequenceComparer.Equals(left, right); + bool expected = left.AsSpan().SequenceEqual(right.AsSpan()); Assert.Equal(expected, builder1.ContentEquals(builder2)); } From 9483228d20c73ba66d66d06705f2742eccdca575 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Tue, 24 Jan 2023 20:39:38 +0200 Subject: [PATCH 5/7] Fix compiler errors. --- .../System/Reflection/Internal/Utilities/BlobDictionary.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/BlobDictionary.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/BlobDictionary.cs index 968279e053c3da..81997fc8bef4a4 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/BlobDictionary.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/BlobDictionary.cs @@ -21,7 +21,7 @@ private static int GetNextDictionaryKey(int dictionaryKey) => (int)((uint)dictionaryKey * 747796405 + 2891336453); #if NET - private ref KeyValuePair, TValue> GetValueRefOrAddDefault(ReadOnlySpan key, out bool exists) + private unsafe ref KeyValuePair, TValue> GetValueRefOrAddDefault(ReadOnlySpan key, out bool exists) { int dictionaryKey = Hash.GetFNVHashCode(key); while (true) @@ -29,7 +29,11 @@ private ref KeyValuePair, TValue> GetValueRefOrAddDefault(R ref var entry = ref CollectionsMarshal.GetValueRefOrAddDefault(_dictionary, dictionaryKey, out exists); if (!exists || entry.Key.AsSpan().SequenceEqual(key)) { +#pragma warning disable CS9082 // Local is returned by reference but was initialized to a value that cannot be returned by reference + // In .NET 6 the assembly of GetValueRefOrAddDefault was compiled with earlier ref safety rules + // and caused an error, which was turned into a warning because of unsafe and was suppressed. return ref entry; +#pragma warning restore CS9082 } dictionaryKey = GetNextDictionaryKey(dictionaryKey); } From bb40c67ba3062e982d90b3ba90a25c3e92ec7795 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Tue, 24 Jan 2023 21:14:05 +0200 Subject: [PATCH 6/7] Clean-up `BlobDictionary`; move it, make it non-generic and streamline its API. --- .../src/System.Reflection.Metadata.csproj | 2 +- .../Ecma335}/BlobDictionary.cs | 36 +++++++++++-------- .../Metadata/Ecma335/MetadataBuilder.Heaps.cs | 17 +++------ 3 files changed, 27 insertions(+), 28 deletions(-) rename src/libraries/System.Reflection.Metadata/src/System/Reflection/{Internal/Utilities => Metadata/Ecma335}/BlobDictionary.cs (72%) diff --git a/src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj b/src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj index 6b2539dd8f85e3..a493c1db2963ed 100644 --- a/src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj +++ b/src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj @@ -23,7 +23,6 @@ The System.Reflection.Metadata library is built-in as part of the shared framewo - @@ -60,6 +59,7 @@ The System.Reflection.Metadata library is built-in as part of the shared framewo + diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/BlobDictionary.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/BlobDictionary.cs similarity index 72% rename from src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/BlobDictionary.cs rename to src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/BlobDictionary.cs index 81997fc8bef4a4..0fa272a27062b1 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/BlobDictionary.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/BlobDictionary.cs @@ -4,16 +4,17 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; +using System.Reflection.Internal; #if NET using System.Runtime.InteropServices; #endif -namespace System.Reflection.Internal +namespace System.Reflection.Metadata.Ecma335 { [DebuggerDisplay("Count = {Count}")] - internal readonly struct BlobDictionary + internal readonly struct BlobDictionary { - private readonly Dictionary, TValue>> _dictionary; + private readonly Dictionary, BlobHandle>> _dictionary; // A simple LCG. Constants taken from // https://github.com/imneme/pcg-c/blob/83252d9c23df9c82ecb42210afed61a7b42402d7/include/pcg_variants.h#L276-L284 @@ -21,7 +22,7 @@ private static int GetNextDictionaryKey(int dictionaryKey) => (int)((uint)dictionaryKey * 747796405 + 2891336453); #if NET - private unsafe ref KeyValuePair, TValue> GetValueRefOrAddDefault(ReadOnlySpan key, out bool exists) + private unsafe ref KeyValuePair, BlobHandle> GetValueRefOrAddDefault(ReadOnlySpan key, out bool exists) { int dictionaryKey = Hash.GetFNVHashCode(key); while (true) @@ -39,7 +40,7 @@ private unsafe ref KeyValuePair, TValue> GetValueRefOrAddDe } } - private TValue GetOrAdd(ReadOnlySpan key, ImmutableArray immutableKey, TValue value, out bool exists) + public BlobHandle GetOrAdd(ReadOnlySpan key, ImmutableArray immutableKey, BlobHandle value, out bool exists) { ref var entry = ref GetValueRefOrAddDefault(key, out exists); if (exists) @@ -47,18 +48,24 @@ private TValue GetOrAdd(ReadOnlySpan key, ImmutableArray immutableKe return entry.Value; } + // If we are given an immutable array, do not allocate a new one. if (immutableKey.IsDefault) { immutableKey = key.ToImmutableArray(); } + else + { + Debug.Assert(immutableKey.AsSpan().SequenceEqual(key)); + } + entry = new(immutableKey, value); return value; } #else - private TValue GetOrAdd(ReadOnlySpan key, ImmutableArray immutableKey, TValue value, out bool exists) + public BlobHandle GetOrAdd(ReadOnlySpan key, ImmutableArray immutableKey, BlobHandle value, out bool exists) { int dictionarykey = Hash.GetFNVHashCode(key); - KeyValuePair, TValue> entry; + KeyValuePair, BlobHandle> entry; while (true) { if (!(exists = _dictionary.TryGetValue(dictionarykey, out entry)) @@ -74,10 +81,16 @@ private TValue GetOrAdd(ReadOnlySpan key, ImmutableArray immutableKe return entry.Value; } + // If we are given an immutable array, do not allocate a new one. if (immutableKey.IsDefault) { immutableKey = key.ToImmutableArray(); } + else + { + Debug.Assert(immutableKey.AsSpan().SequenceEqual(key)); + } + _dictionary.Add(dictionarykey, new(immutableKey, value)); return value; } @@ -90,14 +103,7 @@ public BlobDictionary(int capacity = 0) public int Count => _dictionary.Count; - public IEnumerator, TValue>>> GetEnumerator() => + public Dictionary, BlobHandle>>.Enumerator GetEnumerator() => _dictionary.GetEnumerator(); - - public TValue GetOrAdd(ReadOnlySpan key, TValue value, out bool exists) => - GetOrAdd(key, default, value, out exists); - - // If we are given an immutable array, do not allocate a new one. - public TValue GetOrAdd(ImmutableArray key, TValue value, out bool exists) => - GetOrAdd(key.AsSpan(), key, value, out exists); } } diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataBuilder.Heaps.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataBuilder.Heaps.cs index 65df9df609e7a3..d0862f9cbbd11e 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataBuilder.Heaps.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataBuilder.Heaps.cs @@ -42,7 +42,7 @@ internal void SetCapacity(int capacity) private int _stringHeapCapacity = 4 * 1024; // #Blob heap - private readonly BlobDictionary _blobs = new BlobDictionary(1024); + private readonly BlobDictionary _blobs = new BlobDictionary(1024); private readonly int _blobHeapStartOffset; private int _blobHeapSize; @@ -116,7 +116,7 @@ public MetadataBuilder( // beginning of the delta blob. _userStringBuilder.WriteByte(0); - _blobs.GetOrAdd(ImmutableArray.Empty, default, out _); + _blobs.GetOrAdd(ReadOnlySpan.Empty, ImmutableArray.Empty, default, out _); _blobHeapSize = 1; // When EnC delta is applied #US, #String and #Blob heaps are appended. @@ -215,10 +215,10 @@ public BlobHandle GetOrAddBlob(byte[] value) return GetOrAddBlob(new ReadOnlySpan(value)); } - private BlobHandle GetOrAddBlob(ReadOnlySpan value) + private BlobHandle GetOrAddBlob(ReadOnlySpan value, ImmutableArray immutableValue = default) { BlobHandle nextHandle = BlobHandle.FromOffset(_blobHeapStartOffset + _blobHeapSize); - BlobHandle handle = _blobs.GetOrAdd(value, nextHandle, out bool exists); + BlobHandle handle = _blobs.GetOrAdd(value, immutableValue, nextHandle, out bool exists); if (!exists) { _blobHeapSize += BlobWriterImpl.GetCompressedIntegerSize(value.Length) + value.Length; @@ -240,14 +240,7 @@ public BlobHandle GetOrAddBlob(ImmutableArray value) Throw.ArgumentNull(nameof(value)); } - BlobHandle nextHandle = BlobHandle.FromOffset(_blobHeapStartOffset + _blobHeapSize); - BlobHandle handle = _blobs.GetOrAdd(value, nextHandle, out bool exists); - if (!exists) - { - _blobHeapSize += BlobWriterImpl.GetCompressedIntegerSize(value.Length) + value.Length; - } - - return handle; + return GetOrAddBlob(value.AsSpan(), value); } /// From 9c62c03c156c136afa6bfb66fa06fcaf1287c51a Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Wed, 12 Apr 2023 01:23:11 +0300 Subject: [PATCH 7/7] Don't try to copy the blob builder's contents to a stack-allocated buffer. The blob builder's default starting size is 256 bytes; the same as the stack-alllocated buffer, and making it bigger does not have any benefit since such large blobs are rare. --- .../System/Reflection/Metadata/BlobBuilder.cs | 16 +--------------- .../Metadata/Ecma335/MetadataBuilder.Heaps.cs | 2 +- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/BlobBuilder.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/BlobBuilder.cs index 4ad7d70e4b3053..98bda9ebddd8e7 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/BlobBuilder.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/BlobBuilder.cs @@ -319,7 +319,7 @@ public ImmutableArray ToImmutableArray(int start, int byteCount) return ImmutableByteArrayInterop.DangerousCreateFromUnderlyingArray(ref array); } - internal bool TryWriteTo(Span scratchBuffer, out ReadOnlySpan buffer) + internal bool TryGetSpan(out ReadOnlySpan buffer) { if (_nextOrPrevious == this) { @@ -328,20 +328,6 @@ internal bool TryWriteTo(Span scratchBuffer, out ReadOnlySpan buffer return true; } - int count = Count; - if (scratchBuffer.Length <= count) - { - int i = 0; - foreach (var chunk in GetChunks()) - { - chunk.Span.CopyTo(scratchBuffer.Slice(i)); - i += chunk.Length; - } - - buffer = scratchBuffer.Slice(0, count); - return true; - } - buffer = default; return false; } diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataBuilder.Heaps.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataBuilder.Heaps.cs index d0862f9cbbd11e..142ea5f67c6f99 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataBuilder.Heaps.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataBuilder.Heaps.cs @@ -191,7 +191,7 @@ public BlobHandle GetOrAddBlob(BlobBuilder value) Throw.ArgumentNull(nameof(value)); } - if (value.TryWriteTo(stackalloc byte[256], out ReadOnlySpan buffer)) + if (value.TryGetSpan(out ReadOnlySpan buffer)) { return GetOrAddBlob(buffer); }