Replace MemoryMarshal generic Read/Write with BitConverter equivalents#127394
Replace MemoryMarshal generic Read/Write with BitConverter equivalents#127394EgorBo merged 7 commits intodotnet:mainfrom
Conversation
Sweep src/libraries and System.Private.CoreLib (excluding tests) to replace MemoryMarshal.Read<T>/Write<T>/TryRead<T>/TryWrite<T> calls on primitive types with BitConverter.To*/TryWriteBytes (preferred, host-endian) or BinaryPrimitives equivalents. The generic MemoryMarshal.* APIs are planned to be marked caller-unsafe. Out of scope (intentionally untouched): struct Read/Write (Guid, IcmpHeader, DbRow, StackRow, Sec_Application_Protocols, ReparseBuffers), generic Read<T>/Write<T> helper methods, AsBytes/Cast/AsRef/GetReference/CreateSpan, and BinaryPrimitives implementations themselves. For projects targeting netstandard2.0/net4x where BitConverter.To*(ReadOnlySpan<byte>) and TryWriteBytes(Span<byte>, T) are not available, BitConverterPolyfills.cs is extended with C# 14 extension(BitConverter) overloads and conditionally included in the relevant csprojs (System.Text.Json, System.Diagnostics.PerformanceCounter, System.Security.Cryptography.Pkcs, System.Formats.Cbor, System.Security.Cryptography.Cose). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Sweep across libraries and System.Private.CoreLib replacing generic MemoryMarshal.Read/Write/TryRead/TryWrite<T> on primitive types with BitConverter span APIs (host-endian), plus adding downlevel BitConverter span polyfills for non-.NETCoreApp targets to support the migration ahead of making MemoryMarshal.*<T> caller-unsafe.
Changes:
- Replaced primitive
MemoryMarshal.Read/Writeusages withBitConverter.To*/BitConverter.TryWriteBytesin several libraries and CoreLib hot paths. - Updated
Booleanparsing fast-path to be endian-independent without relying onMemoryMarshal.Read<ulong>(AsBytes(chars)). - Added/linked
Common/src/Polyfills/BitConverterPolyfills.csinto multi-targeted projects to provide missing span overloads on downlevel TFMs.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/PropertyRef.cs | Uses BitConverter.To* for key materialization from name bytes. |
| src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs | Replaces primitive reads/writes in metadata DB headers with BitConverter APIs. |
| src/libraries/System.Text.Json/src/System.Text.Json.csproj | Links BitConverterPolyfills.cs for non-.NETCoreApp TFMs. |
| src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslCrlCache.cs | Switches URL hash extraction to BitConverter.ToUInt32. |
| src/libraries/System.Security.Cryptography.Pkcs/src/System.Security.Cryptography.Pkcs.csproj | Links BitConverterPolyfills.cs for non-.NETCoreApp TFMs. |
| src/libraries/System.Security.Cryptography.Pkcs/src/Internal/Cryptography/Pal/Windows/HelpersWindows.cs | Uses BitConverter.ToInt32 for interop-returned primitive values. |
| src/libraries/System.Security.Cryptography.Cose/src/System.Security.Cryptography.Cose.csproj | Links BitConverterPolyfills.cs for non-.NETCoreApp TFMs. |
| src/libraries/System.Private.CoreLib/src/System/Random.Xoshiro256StarStarImpl.cs | Uses BitConverter.TryWriteBytes for NextBytes block writes. |
| src/libraries/System.Private.CoreLib/src/System/Random.Xoshiro128StarStarImpl.cs | Uses BitConverter.TryWriteBytes for NextBytes block writes. |
| src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventPipePayloadDecoder.cs | Simplifies byte/sbyte reads to payload[0] / cast. |
| src/libraries/System.Private.CoreLib/src/System/Boolean.cs | Reworks "true"/"false" case-insensitive checks to be endian-independent. |
| src/libraries/System.Private.CoreLib/src/Internal/Win32/RegistryKey.cs | Uses BitConverter.ToInt32/ToInt64 for registry numeric values. |
| src/libraries/System.Net.Primitives/src/System/Net/IPAddress.cs | Uses BitConverter for IPv4 constructor read, write, and hash computation reads. |
| src/libraries/System.Formats.Cbor/src/System/Formats/Cbor/CborHelpers.netstandard.cs | Uses BitConverter.To* / TryWriteBytes for big-endian float helpers on downlevel TFMs. |
| src/libraries/System.Formats.Cbor/src/System.Formats.Cbor.csproj | Links BitConverterPolyfills.cs for non-.NETCoreApp TFMs. |
| src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Windows.cs | Uses BitConverter.ToInt32/ToInt64 for perf counter reads. |
| src/libraries/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs | Uses BitConverter.ToUInt32/ToInt64 for perf counter data reads. |
| src/libraries/System.Diagnostics.PerformanceCounter/src/System.Diagnostics.PerformanceCounter.csproj | Links BitConverterPolyfills.cs for non-.NETCoreApp TFMs. |
| src/libraries/Microsoft.Win32.Registry/src/Microsoft/Win32/RegistryKey.cs | Uses BitConverter.ToInt32/ToInt64 for registry numeric values. |
| src/libraries/Common/src/Polyfills/HashCodePolyfills.cs | Uses BitConverter.ToInt32(ReadOnlySpan<byte>) when hashing bytes on downlevel TFMs. |
| src/libraries/Common/src/Polyfills/BitConverterPolyfills.cs | Adds downlevel BitConverter span overloads via extension members. |
…es+Assert Replace the TryWriteBytes-plus-Debug.Assert(ok) pattern (which obscures the contract) with the throwing BinaryPrimitives.Write*LittleEndian / Write*BigEndian helpers, which match the exception semantics of the original MemoryMarshal.Write calls. CborHelpers.netstandard.cs is simplified by replacing the BitConverter.IsLittleEndian + ReverseEndianness ternary with direct BinaryPrimitives.*BigEndian calls. The TryWriteBytes overloads in BitConverterPolyfills.cs are no longer needed and have been removed; only the ReadOnlySpan<byte> read overloads remain. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…s touch The polyfill itself is internal-only and we don't care if it uses generic MemoryMarshal.Read<T> — the goal of marking those caller-unsafe is for the public API surface. Switching the polyfill body to MemoryMarshal.Read collapses each overload to a one-liner. The Boolean.cs change is reverted: keep the existing MemoryMarshal.Read<ulong>(MemoryMarshal.AsBytes(value)) form. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates multiple libraries and System.Private.CoreLib to avoid generic MemoryMarshal.Read<T>/Write<T>/TryRead<T>/TryWrite<T> on primitive types, replacing them with BitConverter (and in a few places BinaryPrimitives) equivalents as preparation for making the generic MemoryMarshal.* APIs caller-unsafe.
Changes:
- Replace primitive
MemoryMarshal.Read/Writeusage withBitConverter.To*/BitConverter.TryWriteBytes-style APIs (plus downlevel polyfills where required). - Add
BitConverterspan polyfill inclusion to several non-.NETCoreApp project files. - Simplify some endian-specific helper logic by using
BinaryPrimitivesbig-/little-endian helpers.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/PropertyRef.cs | Switches primitive span reads from MemoryMarshal.Read<T> to BitConverter.To*. |
| src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs | Changes several row-field reads/writes to BinaryPrimitives.*LittleEndian. |
| src/libraries/System.Text.Json/src/System.Text.Json.csproj | Includes BitConverterPolyfills.cs for non-.NETCoreApp builds. |
| src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslCrlCache.cs | Uses BitConverter.ToUInt32 instead of MemoryMarshal.Read<uint>. |
| src/libraries/System.Security.Cryptography.Pkcs/src/System.Security.Cryptography.Pkcs.csproj | Conditionally includes BitConverterPolyfills.cs for non-.NETCoreApp builds. |
| src/libraries/System.Security.Cryptography.Pkcs/src/Internal/Cryptography/Pal/Windows/HelpersWindows.cs | Uses BitConverter.ToInt32 instead of MemoryMarshal.Read<int>. |
| src/libraries/System.Security.Cryptography.Cose/src/System.Security.Cryptography.Cose.csproj | Conditionally includes BitConverterPolyfills.cs for non-.NETCoreApp builds. |
| src/libraries/System.Private.CoreLib/src/System/Random.Xoshiro256StarStarImpl.cs | Changes PRNG block writes from host-endian to WriteUInt64LittleEndian. |
| src/libraries/System.Private.CoreLib/src/System/Random.Xoshiro128StarStarImpl.cs | Changes PRNG block writes from host-endian to WriteUInt32LittleEndian. |
| src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventPipePayloadDecoder.cs | Replaces MemoryMarshal.Read<byte/sbyte> with direct indexing. |
| src/libraries/System.Private.CoreLib/src/Internal/Win32/RegistryKey.cs | Uses BitConverter.ToInt32/ToInt64 instead of MemoryMarshal.Read<int/long>. |
| src/libraries/System.Net.Primitives/src/System/Net/IPAddress.cs | Replaces primitive reads/writes; IPv4 write uses WriteUInt32LittleEndian. |
| src/libraries/System.Formats.Cbor/src/System/Formats/Cbor/CborHelpers.netstandard.cs | Simplifies big-endian read/write helpers using BinaryPrimitives. |
| src/libraries/System.Formats.Cbor/src/System.Formats.Cbor.csproj | Includes BitConverterPolyfills.cs for non-.NETCoreApp builds. |
| src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Windows.cs | Uses BitConverter.ToInt32/ToInt64 for counter reads. |
| src/libraries/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs | Uses BitConverter span overloads for value reads. |
| src/libraries/System.Diagnostics.PerformanceCounter/src/System.Diagnostics.PerformanceCounter.csproj | Conditionally includes BitConverterPolyfills.cs for non-.NETCoreApp builds. |
| src/libraries/Microsoft.Win32.Registry/src/Microsoft/Win32/RegistryKey.cs | Uses BitConverter.ToInt32/ToInt64 instead of MemoryMarshal.Read<int/long>. |
| src/libraries/Common/src/Polyfills/HashCodePolyfills.cs | Switches to BitConverter.ToInt32(ReadOnlySpan<byte>). |
| src/libraries/Common/src/Polyfills/BitConverterPolyfills.cs | Adds downlevel span BitConverter.To* overloads via extension members. |
Both libraries compile their source files only when TargetPlatformIdentifier == 'windows'; on plain net462/ns2.0 the assembly is generated as a PlatformNotSupported facade. Including the polyfill (which uses ReadOnlySpan<byte>) unconditionally for non-NETCoreApp pulled it into the facade build where System.Memory isn't referenced, causing CS0246. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…taDb Copilot review correctly flagged that BinaryPrimitives.*LittleEndian changes behavior on big-endian platforms vs the original MemoryMarshal.Write/Read (which is host-endian). For these specific cases (Random.XoshiroImpl bulk fill, IPAddress.WriteIPv4Bytes, JsonDocument.MetadataDb DbRow accessors), revert to MemoryMarshal.Read/Write to preserve host-endian semantics and fail-fast on insufficient destination length. Drop now-unused System.Buffers.Binary using directives. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates multiple library components to replace generic MemoryMarshal.Read/Write/TryRead/TryWrite usages on primitive types with BitConverter span-based APIs (host-endian), and adds/extends downlevel BitConverter span overload polyfills for non-.NETCoreApp targets.
Changes:
- Replace primitive
MemoryMarshal.Read<T>call sites withBitConverter.To*equivalents across several libraries. - Simplify big-endian helpers in
System.Formats.Cborto useBinaryPrimitives.Read*/Write*BigEndian. - Extend
CommonBitConverterpolyfills withReadOnlySpan<byte>overloads and wire them into impacted non-.NETCoreAppprojects.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/PropertyRef.cs | Switch key extraction reads from MemoryMarshal.Read<*> to BitConverter.To* on spans. |
| src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs | Adjust MemoryMarshal.Write argument passing (ref → in) in metadata DB updates. |
| src/libraries/System.Text.Json/src/System.Text.Json.csproj | Include BitConverterPolyfills.cs for non-.NETCoreApp builds. |
| src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslCrlCache.cs | Use BitConverter.ToUInt32 to interpret 4-byte hash span. |
| src/libraries/System.Security.Cryptography.Pkcs/src/System.Security.Cryptography.Pkcs.csproj | Include BitConverterPolyfills.cs for non-.NETCoreApp Windows builds. |
| src/libraries/System.Security.Cryptography.Pkcs/src/Internal/Cryptography/Pal/Windows/HelpersWindows.cs | Replace primitive reads with BitConverter.ToInt32 on spans. |
| src/libraries/System.Security.Cryptography.Cose/src/System.Security.Cryptography.Cose.csproj | Include BitConverterPolyfills.cs for non-.NETCoreApp builds. |
| src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventPipePayloadDecoder.cs | Replace byte/sbyte MemoryMarshal.Read<T> with direct span indexing. |
| src/libraries/System.Private.CoreLib/src/Internal/Win32/RegistryKey.cs | Use BitConverter.ToInt32/ToInt64 to decode registry integer values. |
| src/libraries/System.Net.Primitives/src/System/Net/IPAddress.cs | Replace primitive span reads in ctor/hash code with BitConverter.ToUInt32. |
| src/libraries/System.Formats.Cbor/src/System/Formats/Cbor/CborHelpers.netstandard.cs | Use BinaryPrimitives big-endian APIs for half/single/double helpers. |
| src/libraries/System.Formats.Cbor/src/System.Formats.Cbor.csproj | Include BitConverterPolyfills.cs for non-.NETCoreApp builds. |
| src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Windows.cs | Replace perf-counter numeric reads with BitConverter.ToInt32/ToInt64. |
| src/libraries/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs | Replace span numeric reads with BitConverter.ToUInt32/ToInt64. |
| src/libraries/System.Diagnostics.PerformanceCounter/src/System.Diagnostics.PerformanceCounter.csproj | Include BitConverterPolyfills.cs for non-.NETCoreApp Windows builds. |
| src/libraries/Microsoft.Win32.Registry/src/Microsoft/Win32/RegistryKey.cs | Replace primitive registry value reads with BitConverter.ToInt32/ToInt64. |
| src/libraries/Common/src/Polyfills/HashCodePolyfills.cs | Switch HashCode.AddBytes chunk reads to BitConverter.ToInt32. |
| src/libraries/Common/src/Polyfills/BitConverterPolyfills.cs | Add span-based BitConverter.To* polyfills implemented via MemoryMarshal.Read<T>. |
netstandard2.0 MemoryMarshal.Write<T>(span, ref T) doesn't have the 'in' overload that netcoreapp does. Switch the JsonDocument.MetadataDb call sites to 'ref' to compile across all TFMs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Note This benchmark request was generated by GitHub Copilot. @EgorBot -linux_amd -windows_amd -osx_arm64 using System;
using System.Buffers.Binary;
using System.IO.Hashing;
using System.Numerics;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
BenchmarkSwitcher.FromAssembly(typeof(SafeSpanRewriteBenchmarks).Assembly).Run(args);
[MemoryDiagnoser]
public class SafeSpanRewriteBenchmarks
{
private byte[] _bytes = null!;
private byte[] _randomBuffer = null!;
private int[] _ints = null!;
private int[] _reversedInts = null!;
private long[] _longs = null!;
private long[] _reversedLongs = null!;
private BigInteger _bigInteger;
private Random _random = null!;
[Params(64, 1024, 4096)]
public int Length { get; set; }
[GlobalSetup]
public void Setup()
{
_bytes = new byte[Length];
_randomBuffer = new byte[Length];
var seed = new Random(12345);
seed.NextBytes(_bytes);
_bytes[^1] |= 1;
_ints = new int[Math.Max(1, Length / sizeof(int))];
_reversedInts = new int[_ints.Length];
for (int i = 0; i < _ints.Length; i++)
{
_ints[i] = i * 0x01020304 + 0x10203040;
}
_longs = new long[Math.Max(1, Length / sizeof(long))];
_reversedLongs = new long[_longs.Length];
for (int i = 0; i < _longs.Length; i++)
{
_longs[i] = ((long)i << 32) | (uint)(i * 0x01020304 + 0x10203040);
}
_bigInteger = new BigInteger(_bytes, isUnsigned: true);
_random = new Random();
}
[Benchmark]
public int HashCodeAddBytes()
{
HashCode hashCode = default;
hashCode.AddBytes(_bytes);
return hashCode.ToHashCode();
}
[Benchmark]
public int RandomNextBytes()
{
_random.NextBytes(_randomBuffer);
return _randomBuffer[0];
}
[Benchmark]
public uint Adler32HashToUInt32() => Adler32.HashToUInt32(_bytes);
[Benchmark]
public uint Crc32HashToUInt32() => Crc32.HashToUInt32(_bytes);
[Benchmark]
public ulong Crc64HashToUInt64() => Crc64.HashToUInt64(_bytes);
[Benchmark]
public int ReverseEndiannessInt32()
{
BinaryPrimitives.ReverseEndianness(_ints, _reversedInts);
return _reversedInts[0];
}
[Benchmark]
public long ReverseEndiannessInt64()
{
BinaryPrimitives.ReverseEndianness(_longs, _reversedLongs);
return _reversedLongs[0];
}
[Benchmark]
public BigInteger BigIntegerLeftShift() => _bigInteger << 13;
[Benchmark]
public BigInteger BigIntegerRightShift() => _bigInteger >> 13;
[Benchmark]
public BigInteger BigIntegerOnesComplement() => ~_bigInteger;
} |
There was a problem hiding this comment.
Pull request overview
Sweep across src/libraries (and CoreLib) to replace generic MemoryMarshal.Read<T>/Write<T> on primitive types with host-endian BitConverter.* equivalents, and to add downlevel span-based BitConverter polyfills where needed.
Changes:
- Replace
MemoryMarshal.Read<T>usages on primitive types withBitConverter.To*/ direct byte access for 1-byte primitives. - Add
BitConverterspan overload polyfill to downlevel-targeting projects that now callBitConverter.To*(ReadOnlySpan<byte>). - Simplify big-endian CBOR helpers to use
BinaryPrimitives.{Read,Write}*BigEndiandirectly.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/PropertyRef.cs | Switch primitive span reads to BitConverter.ToUInt16/32/64 for key generation. |
| src/libraries/System.Text.Json/src/System.Text.Json.csproj | Include BitConverterPolyfills.cs for non-.NETCoreApp TFMs. |
| src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslCrlCache.cs | Replace MemoryMarshal.Read<uint> with BitConverter.ToUInt32 for URL hash derivation. |
| src/libraries/System.Security.Cryptography.Pkcs/src/System.Security.Cryptography.Pkcs.csproj | Conditionally include BitConverterPolyfills.cs for downlevel Windows builds. |
| src/libraries/System.Security.Cryptography.Pkcs/src/Internal/Cryptography/Pal/Windows/HelpersWindows.cs | Replace primitive reads with BitConverter.ToInt32. |
| src/libraries/System.Security.Cryptography.Cose/src/System.Security.Cryptography.Cose.csproj | Include BitConverterPolyfills.cs for non-.NETCoreApp TFMs. |
| src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventPipePayloadDecoder.cs | Replace 1-byte primitive reads with direct indexing/cast. |
| src/libraries/System.Private.CoreLib/src/Internal/Win32/RegistryKey.cs | Replace primitive registry value reads with BitConverter.ToInt32/ToInt64. |
| src/libraries/System.Net.Primitives/src/System/Net/IPAddress.cs | Replace primitive reads with BitConverter.ToUInt32 / BitConverter.ToUInt32 in IPv6 hash computation. |
| src/libraries/System.Formats.Cbor/src/System/Formats/Cbor/CborHelpers.netstandard.cs | Replace manual endianness handling with BinaryPrimitives.*BigEndian APIs. |
| src/libraries/System.Formats.Cbor/src/System.Formats.Cbor.csproj | Include BitConverterPolyfills.cs for non-.NETCoreApp TFMs. |
| src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Windows.cs | Replace primitive reads with BitConverter.ToInt32/ToInt64. |
| src/libraries/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs | Replace primitive reads with BitConverter.ToUInt32/ToInt64. |
| src/libraries/System.Diagnostics.PerformanceCounter/src/System.Diagnostics.PerformanceCounter.csproj | Conditionally include BitConverterPolyfills.cs for downlevel Windows builds. |
| src/libraries/Microsoft.Win32.Registry/src/Microsoft/Win32/RegistryKey.cs | Replace primitive registry value reads with BitConverter.ToInt32/ToInt64. |
| src/libraries/Common/src/Polyfills/HashCodePolyfills.cs | Use BitConverter.ToInt32(ReadOnlySpan<byte>) when folding bytes into HashCode. |
| src/libraries/Common/src/Polyfills/BitConverterPolyfills.cs | Add downlevel span-based BitConverter.To* overloads via extension(BitConverter) members. |
Avoid MemoryMarshal.Read/Write because these APIs will marked as caller-unsafe. Use safer alternatives.