[SRM] Optimize `MetadataBuilder.GetOrAddConstantBlob`. by teo-tsirpanis · Pull Request #121223 · dotnet/runtime

teo-tsirpanis · 2025-10-31T00:03:44Z

In MetadataBuilder.GetOrAddConstantBlob, the pooled blob builder was replaced by writing to a stack-allocated span, resulting in a 2x performance improvement when repeatedly writing the same scalar value (see comments in the bottom for benchmark results).

The logic of writing scalar constants was extracted to its own method that writes to a span. This method also got used by `MetadataBuilder.GetOrAddConstantBlob`, avoiding the use of pooled blob builders when writing scalar constants.

dotnet-policy-service · 2025-10-31T00:04:40Z

Tagging subscribers to this area: @dotnet/area-system-reflection-metadata
See info in area-owners.md if you want to be subscribed.

Copilot

Pull Request Overview

This PR optimizes constant blob encoding by eliminating unnecessary allocations. It refactors the GetOrAddConstantBlob method and the WriteConstant methods to use stack-allocated buffers instead of pooled BlobBuilder instances for scalar (non-string) constants.

Key Changes:

Introduced a new WriteScalarConstant method that writes scalar constants to a Span<byte> and returns the number of bytes written
Refactored GetOrAddConstantBlob to use stack allocation for scalar values
Updated both WriteConstant overloads to call WriteScalarConstant for non-string values

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
MetadataBuilder.Heaps.cs	Refactored `GetOrAddConstantBlob` to use stack-allocated buffer and new `WriteScalarConstant` method
BlobWriterImpl.cs	Added new `WriteScalarConstant` method and refactored existing `WriteConstant` overloads to use it

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

AaronRobinsonMSFT

This looks good to me. Nothing obvious jumps out and I assume we have good test coverage for this area.

jkotas · 2026-03-17T06:21:25Z

optimizes

Do you have any perf numbers that show the benefit of this optimization?

jkotas · 2026-03-18T02:40:20Z

MetadataBuilder.GetOrAddConstantBlob will be faster because of the use of stack-allocated memory instead of pooled blob builders, but it's a little hard to benchmark this method in isolation because there's no MetadataBuilder.Clear method yet.

GetOrAddConstantBlob is returning interned blob, so I would expect that calling it with the same value multiple times is going to be significantly faster. Am I missing something?

jkotas · 2026-03-18T02:43:33Z

I do not think the benchmark you have shared above is exercising the code changed in this PR.

teo-tsirpanis · 2026-03-18T08:38:57Z

I do not think the benchmark you have shared above is exercising the code changed in this PR.

Oops, sorry; I should stop working on multiple PRs at the same time.

GetOrAddConstantBlob is returning interned blob, so I would expect that calling it with the same value multiple times is going to be significantly faster. Am I missing something?

Yes; the idea is that serializing the blob to check if it exists would be faster, as it would use stack-allocated memory instead of a pooled BlobBuilder.

teo-tsirpanis · 2026-03-18T09:17:39Z

Latest EgorBot results show a 2x slowdown for Blob(Builder|Writer).WriteConstant. There could be an argument made that the code is now deduplicated, and I still believe that MetadataBuilder.GetOrAddConstant will be a bit faster (can't think of any way to measure).

Alternatively, I'd be fine with optimizing just GetOrAddConstant by duplicating BlobWriterImpl.WriteConstant once more, or with closing this.

teo-tsirpanis · 2026-03-18T17:52:18Z

I had an idea on how to benchmark MetadataBuilder.GetOrAddConstantBlob, and will try it later today.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

You can also share your feedback on Copilot code review. Take the survey.

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

...ibraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/BlobDictionary.cs

This reverts commit 3842337.

…tantBlob` is now optimized.

teo-tsirpanis · 2026-03-19T02:02:39Z

I copied the minimum necessary subset of SRM's sources into a separate benchmark project, and compared the old and new implementations of MetadataBuilder.GetOrAddConstantBlob. The new implementation was found to be twice as fast:


BenchmarkDotNet v0.15.8, Windows 11 (10.0.26200.8037/25H2/2025Update/HudsonValley2)
AMD Ryzen 9 9950X 4.30GHz, 1 CPU, 32 logical and 16 physical cores
.NET SDK 11.0.100-preview.1.26104.118
  [Host]     : .NET 10.0.5 (10.0.5, 10.0.526.15411), X64 RyuJIT x86-64-v4
  DefaultJob : .NET 10.0.5 (10.0.5, 10.0.526.15411), X64 RyuJIT x86-64-v4

Method	Mean	Error	StdDev	Ratio	Rank	Gen0	Allocated	Alloc Ratio
Old	489.8 ns	5.24 ns	4.91 ns	1.00	2	0.0019	32 B	1.00
New	239.3 ns	2.58 ns	2.41 ns	0.49	1	0.0019	32 B	1.00

Benchmark sources

I reverted all other parts of the PR and scoped it down to optimizing just MetadataBuilder.GetOrAddConstantBlob. I might take another try at BlobBuilder.WriteConstant in the future.

jkotas · 2026-03-19T02:50:11Z

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

+        /// <param name="bytes">The span where the content will be encoded.</param>
+        /// <param name="value">The constant value.</param>
+        /// <returns>The number of bytes that was written.</returns>
+        internal static int WriteScalarConstant(Span<byte> bytes, object? value)


The existing two copies of this method below are called WriteConstant. Use the same name for all 3 for consistency?

The existing two methods also accept strings besides scalar values.

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

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

You can also share your feedback on Copilot code review. Take the survey.

teo-tsirpanis · 2026-03-21T03:06:13Z

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

+        /// <summary>
+        /// Writes a scalar (non-string) constant to a span.
+        /// </summary>
+        /// <param name="bytes">The span where the content will be encoded.</param>
+        /// <param name="value">The constant value.</param>
+        /// <returns>The number of bytes that was written.</returns>
+        internal static int WriteScalarConstant(Span<byte> bytes, object? value)
+        {
+            if (value == null)
+            {
+                // The encoding of Type for the nullref value for FieldInit is ELEMENT_TYPE_CLASS with a Value of a 32-bit.
+                BinaryPrimitives.WriteUInt32LittleEndian(bytes, 0);
+                return sizeof(uint);
+            }
+
+            var type = value.GetType();
+            if (type.IsEnum)
+            {
+                type = Enum.GetUnderlyingType(type);
+            }
+
+            if (type == typeof(bool))


The existing WriteConstant overloads don't have tests. There is https://github.com/dotnet/runtime/blob/main/src/libraries/System.Reflection.Emit/tests/FieldBuilder/FieldBuilderSetConstant.cs that indirectly tests this function.

teo-tsirpanis · 2026-03-21T03:00:40Z

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

+        {
+            if (value == null)
+            {
+                // The encoding of Type for the nullref value for FieldInit is ELEMENT_TYPE_CLASS with a Value of a 32-bit.


Reworded to match how ECMA-335 states it (II.22.9).

teo-tsirpanis · 2026-03-21T02:51:10Z

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

+        internal static int WriteScalarConstant(Span<byte> bytes, object? value)
+        {
+            if (value == null)
+            {
+                // The encoding of Type for the nullref value for FieldInit is ELEMENT_TYPE_CLASS with a Value of a 32-bit.
+                BinaryPrimitives.WriteUInt32LittleEndian(bytes, 0);
+                return sizeof(uint);
+            }


I don't think the assert is necessary, because we are using range-checked APIs to write to the span. It should be OK for example to pass a 4 byte long span if the value is known to be an int.

jkotas · 2026-03-20T04:11:14Z

@teo-tsirpanis Could you please take a look at the copilot feedback?

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Copilot · 2026-03-21T03:06:26Z

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

+            }
+            else if (type == typeof(char))
+            {
+                BinaryPrimitives.WriteUInt16LittleEndian(bytes, (char)value);


BinaryPrimitives.WriteUInt16LittleEndian takes a ushort value; passing (char)value here relies on a conversion that doesn't exist (char -> ushort is not implicit) and will fail to compile. Cast to ushort (e.g., (ushort)(char)value) before calling WriteUInt16LittleEndian.

Suggested change

BinaryPrimitives.WriteUInt16LittleEndian(bytes, (char)value);

BinaryPrimitives.WriteUInt16LittleEndian(bytes, (ushort)(char)value);

Copilot AI review requested due to automatic review settings October 31, 2025 00:03

github-actions bot added the area-System.Reflection.Metadata label Oct 31, 2025

dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Oct 31, 2025

Copilot AI reviewed Oct 31, 2025

View reviewed changes

src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/BlobWriterImpl.cs Show resolved Hide resolved

src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/BlobWriterImpl.cs Outdated Show resolved Hide resolved

build-analysis bot mentioned this pull request Oct 31, 2025

The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008

Open

3 tasks

AaronRobinsonMSFT reviewed Mar 16, 2026

View reviewed changes

src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/BlobWriterImpl.cs Show resolved Hide resolved

AaronRobinsonMSFT approved these changes Mar 16, 2026

View reviewed changes

Use constant for maximum size of scalar constant.

0635302

This was referenced Mar 17, 2026

iOS.Device test WorkItemExecutions #122874

Open

XHarness package install failure on iOS due to devicectl NSPOSIXErrorDomain error 49 #123796

Open

This comment was marked as off-topic.

Sign in to view

EgorBot mentioned this pull request Mar 17, 2026

Benchmarks for dotnet/runtime#121223 (for @teo-tsirpanis) EgorBot/Benchmarks#45

Open

This comment was marked as off-topic.

Sign in to view

Temporarily add method in MetadataBuilder to clear all blobs.

3842337

Copilot AI review requested due to automatic review settings March 18, 2026 23:43

Copilot AI reviewed Mar 18, 2026

View reviewed changes

...s/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataBuilder.Heaps.cs Outdated Show resolved Hide resolved

...ibraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/BlobDictionary.cs Outdated Show resolved Hide resolved

teo-tsirpanis added 3 commits March 19, 2026 03:43

Revert "Temporarily add method in MetadataBuilder to clear all blobs."

89e2166

This reverts commit 3842337.

Revert changes to WriteConstant; only `MetadataBuilder.GetOrAddCons…

b054b30

…tantBlob` is now optimized.

Merge branch 'main' into srm-write-constant

5378ecb

teo-tsirpanis changed the title ~~[SRM] Deduplicate and optimize BlobWriterImpl.WriteConstant.~~ [SRM] Optimize MetadataBuilder.GetOrAddConstantBlob. Mar 19, 2026

jkotas reviewed Mar 19, 2026

View reviewed changes

This was referenced Mar 19, 2026

wasm build failure in CI #117017

Open

osx builds failing with CMake error #125757

Closed

Remove calls to GetTypeInfo().

40944d1

Copilot AI review requested due to automatic review settings March 19, 2026 13:47

Copilot AI reviewed Mar 19, 2026

View reviewed changes

This was referenced Mar 19, 2026

Known Test Repository Issue Sometimes Misses Test Failures dotnet/dnceng#6411

Open

[Mono] mini-llvm.c build failure: LLVMMono1CallConv undeclared after LLVM SDK version bump to 26159.1 #125769

Open

Merge branch 'main' into srm-write-constant

259e337

Update comment to match ECMA-335 wording.

79987b4

Copilot AI review requested due to automatic review settings March 21, 2026 03:00

Copilot started reviewing on behalf of teo-tsirpanis March 21, 2026 03:01 View session

Copilot AI reviewed Mar 21, 2026

View reviewed changes

	BinaryPrimitives.WriteUInt16LittleEndian(bytes, (char)value);
	BinaryPrimitives.WriteUInt16LittleEndian(bytes, (ushort)(char)value);

Conversation

teo-tsirpanis commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dotnet-policy-service bot commented Oct 31, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes:

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AaronRobinsonMSFT left a comment

Choose a reason for hiding this comment

Uh oh!

jkotas commented Mar 17, 2026

Uh oh!

This comment was marked as off-topic.

This comment was marked as off-topic.

jkotas commented Mar 18, 2026

Uh oh!

jkotas commented Mar 18, 2026

Uh oh!

teo-tsirpanis commented Mar 18, 2026

Uh oh!

teo-tsirpanis commented Mar 18, 2026

Uh oh!

teo-tsirpanis commented Mar 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

teo-tsirpanis commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkotas Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

teo-tsirpanis Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

teo-tsirpanis Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

teo-tsirpanis Mar 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teo-tsirpanis Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

jkotas commented Mar 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

teo-tsirpanis commented Oct 31, 2025 •

edited

Loading

teo-tsirpanis commented Mar 19, 2026 •

edited

Loading

teo-tsirpanis Mar 21, 2026 •

edited

Loading