Update vendored System.Runtime.CompilerServices.Unsafe#8469
Merged
Conversation
andrewlock
added
area:vendors
andrewlock
commented
Apr 16, 2026
Comment on lines
-46
to
-48
| <!-- https://www.nuget.org/packages/System.Private.CoreLib/1.0.0 --> | ||
| <PackageReference Include="System.Private.CoreLib" Version="1.0.0" /> | ||
|
|
Member
Author
There was a problem hiding this comment.
This should be in the previous commit, but meh 😅
andrewlock
force-pushed
the
andrew/update-vendors-4
branch
from
April 16, 2026 13:16
bdf65ae to
ef58938
Compare
andrewlock
force-pushed
the
andrew/update-vendors-7
branch
from
April 16, 2026 13:16
0ec03d2 to
3af08ab
Compare
BenchmarksBenchmark execution time: 2026-04-23 10:32:28 Comparing candidate commit 112589a in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 27 metrics, 0 unstable metrics, 60 known flaky benchmarks, 27 flaky benchmarks without significant changes.
|
This was referenced Apr 17, 2026
dudikeleti
approved these changes
Apr 20, 2026
andrewlock
force-pushed
the
andrew/update-vendors-4
branch
from
April 21, 2026 10:19
ef58938 to
7dc44a2
Compare
andrewlock
force-pushed
the
andrew/update-vendors-7
branch
from
April 21, 2026 10:19
3af08ab to
528b407
Compare
andrewlock
force-pushed
the
andrew/update-vendors-4
branch
from
April 23, 2026 07:20
7dc44a2 to
481fbc2
Compare
andrewlock
force-pushed
the
andrew/update-vendors-7
branch
from
April 23, 2026 09:27
528b407 to
112589a
Compare
andrewlock
enabled auto-merge (squash)
April 23, 2026 09:27
NachoEchevarria
approved these changes
Apr 23, 2026
NachoEchevarria
left a comment
NachoEchevarria
left a comment
Collaborator
There was a problem hiding this comment.
Nice! Quick question: is the ultimate goal to stop using dotnet-vendored-code eventually?
andrewlock
added a commit
that referenced
this pull request
Apr 24, 2026
#8476) ## Summary of changes Updates the System.Memory vendoring code to move types like `ReadOnlySpan<T>`, `Span<T>` into `System` instead of `Datadog.Trace.VendoredMicrosoftCode.System` ## Reason for change The compiler has various functionality that relies on the `Span<T>` (and `ReadOnlySpan<T>`) being available in the `System` namespace. By making this change, we get the advantage of those types being available. A separate PR will actually update code to use those types, except where changes were required to make it compile in this PR. ## Implementation details Update the vendoring code to stop changing the namespace. ## Test coverage This is the test, if the tests pass, we should be fine. ## Other details Depends on a stack updating our vendored system code - #8391 - #8454 - #8455 - #8459 - #8461 - #8469
andrewlock
added a commit
that referenced
this pull request
Apr 24, 2026
## Summary of changes Remove some branching code that's no longer required after #8476 moved `Span<T>` to `System` namespace ## Reason for change This sort of stuff is the _reason_ we made that change, to reduce maintenance. ## Implementation details Set 🤖 looking for possible cases, so it's not exhaustive, but gives a taster. I think most of these make sense. It's nothing outstanding but it's the little things. ## Test coverage Just a refactoring, so covered by existing tests. ## Other details By definition, we don't really expect to see performance improvements for this, other than potentially some reduced allocation in .NET Framework. The primary benefits are devx Depends on the vendoring code stack: Depends on a stack updating our vendored system code - #8391 - #8454 - #8455 - #8459 - #8461 - #8469 - #8476 --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
andrewlock
added a commit
that referenced
this pull request
Apr 24, 2026
#8486) ## Summary of changes Update the tag list generator to always use `ReadOnlySpan<byte>` properties instead of `byte[]` ## Reason for change After moving our vendored `Span<T>` implementation into the `System` namespace, various optimizations open up to us, including using `ReadOnlySpan<byte>` properties on .NET Framework instead of `static readonly byte[]` to avoid startup costs. ## Implementation details Replace the code generated by the generator, and update the generated code ## Test coverage Covered by snapshot tests and behaviour is covered by existing tests. We'll check the benchmarks to make sure that we _don't_ see any perf impact (there shouldn't be, impact should just be reduced startup costs) ## Other details Depends on a stack updating our vendored system code - #8391 - #8454 - #8455 - #8459 - #8461 - #8469 - #8476 - #8477
andrewlock
added a commit
that referenced
this pull request
Apr 27, 2026
… string literals (#8487) ## Summary of changes Convert `Encoding.Utf8.GetBytes` calls to static constants using UTF8 string literals ## Reason for change After moving our vendored `Span<T>` implementation into the `System` namespace, various optimizations open up to us, including using UTF-8 string literals, to encode strings to UTF-8 at compile time instead of at runtime. By combing with `static ReadOnlySpan<byte>` properties, these also become zero allocation, so we get reduced overall memory usage as well as better startup time ## Implementation details Replace the following with utf8 string literals: - `StringEncoding.UTF8.GetBytes` - `Encoding.UTF8.GetBytes` - `EncodingHelpers.UTF8NoBom.GetBytes` ## Test coverage All covered by existing tests ## Other details Depends on a stack updating our vendored system code - #8391 - #8454 - #8455 - #8459 - #8461 - #8469 - #8476 - #8477 - #8486
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of changes
Reason for change
We want to be able to update vendored code as required. For the vendored System.Runtime.CompilerServices.Unsafe package, the source code is written directly as IL, and compiled using Ilasm.exe. So this PR introduces a simple "IL to C#" converter that converts that file to its C# equivalent, using the same
InlineIL.Fodyapproach that we currently have.The final result produces a file that is virtually identical to the existing one (which is good!). The only difference I made was to add the original IL line as a comment next to the Fody equivalent.
This also shows that the code has not actually changed (and it's unlikely it will tbh), so this just means we have a repeatable self-contained approach to regenerate this in the repo as required.
Implementation details
Told 🤖 to make the IL to C# converter, and it did 😄 I've given the code it generated a once-over to look for anything terrible, but the key thing is that the output is sane, and that's visibly basically unchanged, so I think it's fine.
Test coverage
@dudikeleti already wrote some extensive tests for the
Unsafeimplementation back when he originally ported it, which verifies that we can compile the methods, call them, and that the generated IL is identical to the "real" versions 🎉Other details
https://datadoghq.atlassian.net/browse/APMLP-1207
Part of a stack updating our vendored system code