Skip to content

Commit 6149ca0

Browse files
Avoid allocations when adding existing items to the blob heap. (#81059)
* Add `BlobDictionary`. * Use `BlobDictionary` in `MetadataReader` and optimize the simple cases. * Optimize adding blobs from UTF-16 strings and from blob builders. * Delete `ByteSequenceComparer`. * Fix compiler errors. * Clean-up `BlobDictionary`; move it, make it non-generic and streamline its API. * 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.
1 parent 71be885 commit 6149ca0

File tree

7 files changed

+163
-94
lines changed

7 files changed

+163
-94
lines changed

src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ The System.Reflection.Metadata library is built-in as part of the shared framewo
4545
<Compile Include="System\Reflection\Metadata\BlobWriterImpl.cs" />
4646
<Compile Include="System\Reflection\Metadata\BlobBuilder.cs" />
4747
<Compile Include="System\Reflection\Metadata\BlobBuilder.Enumerators.cs" />
48-
<Compile Include="System\Reflection\Internal\Utilities\ByteSequenceComparer.cs" />
4948
<Compile Include="System\Reflection\Internal\Utilities\DecimalUtilities.cs" />
5049
<Compile Include="System\Reflection\Internal\Utilities\EnumerableExtensions.cs" />
5150
<Compile Include="System\Reflection\Metadata\Ecma335\CustomAttributeDecoder.cs" />
@@ -60,6 +59,7 @@ The System.Reflection.Metadata library is built-in as part of the shared framewo
6059
<Compile Include="System\Reflection\Metadata\Ecma335\Encoding\InstructionEncoder.cs" />
6160
<Compile Include="System\Reflection\Metadata\Ecma335\Encoding\LabelHandle.cs" />
6261
<Compile Include="System\Reflection\Metadata\IL\ILOpCode.cs" />
62+
<Compile Include="System\Reflection\Metadata\Ecma335\BlobDictionary.cs" />
6363
<Compile Include="System\Reflection\Metadata\Ecma335\CodedIndex.cs" />
6464
<Compile Include="System\Reflection\Metadata\Ecma335\PortablePdbBuilder.cs" />
6565
<Compile Include="System\Reflection\Metadata\Ecma335\MetadataBuilder.Tables.cs" />

src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/ByteSequenceComparer.cs

Lines changed: 0 additions & 68 deletions
This file was deleted.

src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/BlobBuilder.cs

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ public partial class BlobBuilder
5050
private bool IsHead => (_length & IsFrozenMask) == 0;
5151
private int Length => (int)(_length & ~IsFrozenMask);
5252
private uint FrozenLength => _length | IsFrozenMask;
53+
private Span<byte> Span => _buffer.AsSpan(0, Length);
5354

5455
public BlobBuilder(int capacity = DefaultChunkSize)
5556
{
@@ -234,7 +235,7 @@ public bool ContentEquals(BlobBuilder other)
234235
var right = rightEnumerator.Current;
235236

236237
int minLength = Math.Min(left.Length - leftStart, right.Length - rightStart);
237-
if (!ByteSequenceComparer.Equals(left._buffer, leftStart, right._buffer, rightStart, minLength))
238+
if (!left._buffer.AsSpan(leftStart, minLength).SequenceEqual(right._buffer.AsSpan(rightStart, minLength)))
238239
{
239240
return false;
240241
}
@@ -318,6 +319,19 @@ public ImmutableArray<byte> ToImmutableArray(int start, int byteCount)
318319
return ImmutableByteArrayInterop.DangerousCreateFromUnderlyingArray(ref array);
319320
}
320321

322+
internal bool TryGetSpan(out ReadOnlySpan<byte> buffer)
323+
{
324+
if (_nextOrPrevious == this)
325+
{
326+
// If the blob builder has one chunk, we can just return it and avoid copies.
327+
buffer = Span;
328+
return true;
329+
}
330+
331+
buffer = default;
332+
return false;
333+
}
334+
321335
/// <exception cref="ArgumentNullException"><paramref name="destination"/> is null.</exception>
322336
/// <exception cref="InvalidOperationException">Content is not available, the builder has been linked with another one.</exception>
323337
public void WriteContentTo(Stream destination)
@@ -344,7 +358,7 @@ public void WriteContentTo(ref BlobWriter destination)
344358

345359
foreach (var chunk in GetChunks())
346360
{
347-
destination.WriteBytes(chunk._buffer.AsSpan(0, chunk.Length));
361+
destination.WriteBytes(chunk.Span);
348362
}
349363
}
350364

@@ -359,7 +373,7 @@ public void WriteContentTo(BlobBuilder destination)
359373

360374
foreach (var chunk in GetChunks())
361375
{
362-
destination.WriteBytes(chunk._buffer.AsSpan(0, chunk.Length));
376+
destination.WriteBytes(chunk.Span);
363377
}
364378
}
365379

src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/BlobWriter.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public BlobWriter(byte[] buffer, int start, int count)
5454
/// </summary>
5555
public bool ContentEquals(BlobWriter other)
5656
{
57-
return Length == other.Length && ByteSequenceComparer.Equals(_buffer, _start, other._buffer, other._start, Length);
57+
return Length == other.Length && _buffer.AsSpan(_start, Length).SequenceEqual(other._buffer.AsSpan(other._start, other.Length));
5858
}
5959

6060
public int Offset
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System.Collections.Generic;
5+
using System.Collections.Immutable;
6+
using System.Diagnostics;
7+
using System.Reflection.Internal;
8+
#if NET
9+
using System.Runtime.InteropServices;
10+
#endif
11+
12+
namespace System.Reflection.Metadata.Ecma335
13+
{
14+
[DebuggerDisplay("Count = {Count}")]
15+
internal readonly struct BlobDictionary
16+
{
17+
private readonly Dictionary<int, KeyValuePair<ImmutableArray<byte>, BlobHandle>> _dictionary;
18+
19+
// A simple LCG. Constants taken from
20+
// https://github.com/imneme/pcg-c/blob/83252d9c23df9c82ecb42210afed61a7b42402d7/include/pcg_variants.h#L276-L284
21+
private static int GetNextDictionaryKey(int dictionaryKey) =>
22+
(int)((uint)dictionaryKey * 747796405 + 2891336453);
23+
24+
#if NET
25+
private unsafe ref KeyValuePair<ImmutableArray<byte>, BlobHandle> GetValueRefOrAddDefault(ReadOnlySpan<byte> key, out bool exists)
26+
{
27+
int dictionaryKey = Hash.GetFNVHashCode(key);
28+
while (true)
29+
{
30+
ref var entry = ref CollectionsMarshal.GetValueRefOrAddDefault(_dictionary, dictionaryKey, out exists);
31+
if (!exists || entry.Key.AsSpan().SequenceEqual(key))
32+
{
33+
#pragma warning disable CS9082 // Local is returned by reference but was initialized to a value that cannot be returned by reference
34+
// In .NET 6 the assembly of GetValueRefOrAddDefault was compiled with earlier ref safety rules
35+
// and caused an error, which was turned into a warning because of unsafe and was suppressed.
36+
return ref entry;
37+
#pragma warning restore CS9082
38+
}
39+
dictionaryKey = GetNextDictionaryKey(dictionaryKey);
40+
}
41+
}
42+
43+
public BlobHandle GetOrAdd(ReadOnlySpan<byte> key, ImmutableArray<byte> immutableKey, BlobHandle value, out bool exists)
44+
{
45+
ref var entry = ref GetValueRefOrAddDefault(key, out exists);
46+
if (exists)
47+
{
48+
return entry.Value;
49+
}
50+
51+
// If we are given an immutable array, do not allocate a new one.
52+
if (immutableKey.IsDefault)
53+
{
54+
immutableKey = key.ToImmutableArray();
55+
}
56+
else
57+
{
58+
Debug.Assert(immutableKey.AsSpan().SequenceEqual(key));
59+
}
60+
61+
entry = new(immutableKey, value);
62+
return value;
63+
}
64+
#else
65+
public BlobHandle GetOrAdd(ReadOnlySpan<byte> key, ImmutableArray<byte> immutableKey, BlobHandle value, out bool exists)
66+
{
67+
int dictionarykey = Hash.GetFNVHashCode(key);
68+
KeyValuePair<ImmutableArray<byte>, BlobHandle> entry;
69+
while (true)
70+
{
71+
if (!(exists = _dictionary.TryGetValue(dictionarykey, out entry))
72+
|| entry.Key.AsSpan().SequenceEqual(key))
73+
{
74+
break;
75+
}
76+
dictionarykey = GetNextDictionaryKey(dictionarykey);
77+
}
78+
79+
if (exists)
80+
{
81+
return entry.Value;
82+
}
83+
84+
// If we are given an immutable array, do not allocate a new one.
85+
if (immutableKey.IsDefault)
86+
{
87+
immutableKey = key.ToImmutableArray();
88+
}
89+
else
90+
{
91+
Debug.Assert(immutableKey.AsSpan().SequenceEqual(key));
92+
}
93+
94+
_dictionary.Add(dictionarykey, new(immutableKey, value));
95+
return value;
96+
}
97+
#endif
98+
99+
public BlobDictionary(int capacity = 0)
100+
{
101+
_dictionary = new(capacity);
102+
}
103+
104+
public int Count => _dictionary.Count;
105+
106+
public Dictionary<int, KeyValuePair<ImmutableArray<byte>, BlobHandle>>.Enumerator GetEnumerator() =>
107+
_dictionary.GetEnumerator();
108+
}
109+
}

src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataBuilder.Heaps.cs

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,8 @@
33

44
using System.Collections.Generic;
55
using System.Collections.Immutable;
6-
using System.Diagnostics;
76
using System.Reflection.Internal;
8-
using System.Runtime.CompilerServices;
9-
using System.Text;
7+
using System.Runtime.InteropServices;
108

119
namespace System.Reflection.Metadata.Ecma335
1210
{
@@ -44,7 +42,7 @@ internal void SetCapacity(int capacity)
4442
private int _stringHeapCapacity = 4 * 1024;
4543

4644
// #Blob heap
47-
private readonly Dictionary<ImmutableArray<byte>, BlobHandle> _blobs = new Dictionary<ImmutableArray<byte>, BlobHandle>(1024, ByteSequenceComparer.Instance);
45+
private readonly BlobDictionary _blobs = new BlobDictionary(1024);
4846
private readonly int _blobHeapStartOffset;
4947
private int _blobHeapSize;
5048

@@ -118,7 +116,7 @@ public MetadataBuilder(
118116
// beginning of the delta blob.
119117
_userStringBuilder.WriteByte(0);
120118

121-
_blobs.Add(ImmutableArray<byte>.Empty, default(BlobHandle));
119+
_blobs.GetOrAdd(ReadOnlySpan<byte>.Empty, ImmutableArray<byte>.Empty, default, out _);
122120
_blobHeapSize = 1;
123121

124122
// When EnC delta is applied #US, #String and #Blob heaps are appended.
@@ -193,7 +191,11 @@ public BlobHandle GetOrAddBlob(BlobBuilder value)
193191
Throw.ArgumentNull(nameof(value));
194192
}
195193

196-
// TODO: avoid making a copy if the blob exists in the index
194+
if (value.TryGetSpan(out ReadOnlySpan<byte> buffer))
195+
{
196+
return GetOrAddBlob(buffer);
197+
}
198+
197199
return GetOrAddBlob(value.ToImmutableArray());
198200
}
199201

@@ -210,8 +212,19 @@ public BlobHandle GetOrAddBlob(byte[] value)
210212
Throw.ArgumentNull(nameof(value));
211213
}
212214

213-
// TODO: avoid making a copy if the blob exists in the index
214-
return GetOrAddBlob(ImmutableArray.Create(value));
215+
return GetOrAddBlob(new ReadOnlySpan<byte>(value));
216+
}
217+
218+
private BlobHandle GetOrAddBlob(ReadOnlySpan<byte> value, ImmutableArray<byte> immutableValue = default)
219+
{
220+
BlobHandle nextHandle = BlobHandle.FromOffset(_blobHeapStartOffset + _blobHeapSize);
221+
BlobHandle handle = _blobs.GetOrAdd(value, immutableValue, nextHandle, out bool exists);
222+
if (!exists)
223+
{
224+
_blobHeapSize += BlobWriterImpl.GetCompressedIntegerSize(value.Length) + value.Length;
225+
}
226+
227+
return handle;
215228
}
216229

217230
/// <summary>
@@ -227,16 +240,7 @@ public BlobHandle GetOrAddBlob(ImmutableArray<byte> value)
227240
Throw.ArgumentNull(nameof(value));
228241
}
229242

230-
BlobHandle handle;
231-
if (!_blobs.TryGetValue(value, out handle))
232-
{
233-
handle = BlobHandle.FromOffset(_blobHeapStartOffset + _blobHeapSize);
234-
_blobs.Add(value, handle);
235-
236-
_blobHeapSize += BlobWriterImpl.GetCompressedIntegerSize(value.Length) + value.Length;
237-
}
238-
239-
return handle;
243+
return GetOrAddBlob(value.AsSpan(), value);
240244
}
241245

242246
/// <summary>
@@ -267,6 +271,16 @@ public unsafe BlobHandle GetOrAddConstantBlob(object? value)
267271
/// <exception cref="ArgumentNullException"><paramref name="value"/> is null.</exception>
268272
public BlobHandle GetOrAddBlobUTF16(string value)
269273
{
274+
if (value is null)
275+
{
276+
Throw.ArgumentNull(nameof(value));
277+
}
278+
279+
if (BitConverter.IsLittleEndian)
280+
{
281+
return GetOrAddBlob(MemoryMarshal.AsBytes(value.AsSpan()));
282+
}
283+
270284
var builder = PooledBlobBuilder.GetInstance();
271285
builder.WriteUTF16(value);
272286
var handle = GetOrAddBlob(builder);
@@ -611,8 +625,8 @@ private void WriteAlignedBlobHeap(BlobBuilder builder)
611625
int startOffset = _blobHeapStartOffset;
612626
foreach (var entry in _blobs)
613627
{
614-
int heapOffset = entry.Value.GetHeapOffset();
615-
var blob = entry.Key;
628+
int heapOffset = entry.Value.Value.GetHeapOffset();
629+
var blob = entry.Value.Key;
616630

617631
writer.Offset = (heapOffset == 0) ? 0 : heapOffset - startOffset;
618632
writer.WriteCompressedInteger(blob.Length);

src/libraries/System.Reflection.Metadata/tests/Metadata/BlobTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ private void TestContentEquals(byte[] left, byte[] right)
6161
var builder2 = new BlobBuilder(0);
6262
builder2.WriteBytes(right);
6363

64-
bool expected = ByteSequenceComparer.Equals(left, right);
64+
bool expected = left.AsSpan().SequenceEqual(right.AsSpan());
6565
Assert.Equal(expected, builder1.ContentEquals(builder2));
6666
}
6767

0 commit comments

Comments
 (0)