Skip to content

Implement IBinaryInteger<T>.Write*Endian explicitly on the numeric types to avoid boxing - #134152

Closed
caraioniurie47 wants to merge 1 commit into
dotnet:mainfrom
caraioniurie47:issue-77969
Closed

caraioniurie47 wants to merge 1 commit into
dotnet:mainfrom
caraioniurie47:issue-77969

Conversation

@caraioniurie47

Copy link
Copy Markdown
Contributor

IBinaryInteger<T>.WriteBigEndian and WriteLittleEndian (three overloads each) are default interface methods that call TryWriteBigEndian/TryWriteLittleEndian. No implementation provides them, so calling one on a value type through a generic constraint runs the default implementation, which boxes the value unless the JIT removes the box. The repro from the issue, with two lines added to print the bytes allocated by its loop, allocates on main as it does on .NET 10:

Runtime Bytes allocated by the loop (1,000,000 calls)
.NET 10.0.12 24,003,584
main (Release build) 24,002,784
This PR (Release build) 2,776

Change. As suggested in #77969 (comment), the fourteen implementations of IBinaryInteger<T> now implement the six methods explicitly: Byte, SByte, Char, Int16, UInt16, Int32, UInt32, Int64, UInt64, Int128, UInt128, IntPtr, UIntPtr and BigInteger. Each forwards to a new internal BinaryIntegerWriter, whose two generic methods contain the default implementation's body: call the constrained TryWrite method and throw ThrowHelper.ThrowArgumentException_DestinationTooShort() if it returns false. System.Runtime.Numerics compiles the same file, as it already does BigIntegerCalculator.Shared.cs, and gains the matching throw helper and resource string, copied from CoreLib. The Release System.Private.CoreLib.dll grows by 16,384 bytes and System.Runtime.Numerics.dll by 1,024 bytes; no real-world application was measured.

The ref assemblies list the new explicit implementations next to each type's TryWrite* implementations. ApiCompat does not require them: a build with a ref member that the implementation lacked also passed. They are there to keep the ref in step with the implementation, and can be dropped if that is preferred.

Behaviour. Unchanged. The bytes written, the return value and the exceptions (ArgumentException with ParamName destination for a null array, a destination too short and an empty span; ArgumentOutOfRangeException for a startIndex past the end) were compared for all fourteen types on .NET 10.0.12 and on this branch, with the same results. The false path throws the same ArgumentException as the default implementation — Destination is too short. with ParamName destination, from CoreLib's throw helper and, in System.Runtime.Numerics, from the copy of it and of the resource string added here — and in all fourteen types TryWrite*Endian returns false only where the destination is smaller than the value's byte count, so that message describes every failure they report.

Tests. No existing test called these six methods. Each type's *Tests.GenericMath.cs gains three tests next to the TryWrite* tests, all through BinaryIntegerHelper<T>: WriteBigEndianTest and WriteLittleEndianTest check the bytes, the return value and the exceptions, and WriteEndianDoesNotAllocateTest calls the six methods twice and asserts that the second round allocates nothing. With WriteLittleEndian in BinaryIntegerWriter made to return one byte too many, the fourteen WriteLittleEndianTest failed and the fourteen WriteBigEndianTest passed. With the six explicit implementations removed from Int32 and BigInteger, their allocation tests failed (144 and 192 bytes) and the other twelve passed. System.Runtime.Tests and System.Runtime.Numerics.Tests pass.

Left out. IFloatingPoint<T> has twelve default interface methods of the same shape (WriteExponent* and WriteSignificand*), implemented by nine types. I can send them as a follow-up PR.

Performance

On main the JIT already removes the box at many call sites once the code is optimized, so the difference depends on the call site. BenchmarkDotNet 0.15.2, [MemoryDiagnoser], three launches, main and this branch as two Release test hosts that differ only in System.Private.CoreLib.dll and System.Runtime.Numerics.dll. Each call goes through a generic helper, either inlinable or [MethodImpl(MethodImplOptions.NoInlining)]. Mean ± standard deviation; where main's spread is wide its median is given as well, and * marks the rows for which BenchmarkDotNet warned that main's distribution is multimodal.

Case Helper main This PR
long.WriteBigEndian(byte[]) NoInlining 6.64 ± 0.32 ns, 24 B 0.78 ± 0.03 ns, 0 B
BigInteger.WriteBigEndian(Span<byte>) NoInlining 13.60 ± 0.52 ns, 32 B 6.30 ± 0.03 ns, 0 B
long.WriteBigEndian(byte[]) inlinable 0.514 ± 0.009 ns 0.083 ± 0.008 ns
Int128.WriteLittleEndian(Span<byte>) inlinable 0.778 ± 0.024 ns 0.676 ± 0.024 ns
int.WriteLittleEndian(Span<byte>) inlinable 0.252 ± 0.015 ns 0.293 ± 0.012 ns
int.WriteLittleEndian(Span<byte>) NoInlining * 0.75 ± 0.55 ns, median 1.12 1.17 ± 0.01 ns
Int128.WriteLittleEndian(Span<byte>) NoInlining 1.27 ± 0.81 ns, median 1.75 1.63 ± 0.02 ns
BigInteger.WriteBigEndian(Span<byte>) inlinable * 3.34 ± 1.23 ns, median 2.36 4.17 ± 0.05 ns
int.TryWriteLittleEndian(Span<byte>), control NoInlining 1.548 ± 0.018 ns 1.532 ± 0.012 ns

Only the first two rows allocate on main. For the last BigInteger row, which is slower on this branch, DOTNET_JitDisasm of the benchmark method, in a separate one-launch short run, gives the same Tier1 code on both hosts (185 lines, identical apart from addresses and labels). With DOTNET_TieredCompilation=0, main also allocates for the single-argument byte[] overloads and for every BigInteger overload; this branch allocates in none of them.

Resolves #77969

Note

AI-generated, written at my direction and reviewed by me before posting. Everything ran on Windows 11 x64 (build 26200), on local builds of this branch and of main at bad7509cb82, the commit it is based on: the tests and the behaviour comparison on Debug libraries with a Checked runtime, the issue's repro, the benchmark, the disassembly and the DOTNET_TieredCompilation=0 runs on Release builds; the issue's repro and the behaviour comparison also on the installed .NET 10.0.12. System.Runtime.Tests and System.Runtime.Numerics.Tests also passed on an x86 Checked build and on x64 Mono, with its JIT and with MONO_ENV_OPTIONS=--interpreter; nothing ran on NativeAOT, Mono AOT, WASM, Linux or macOS.

The default interface implementations of the six WriteBigEndian and
WriteLittleEndian overloads box a value-type receiver unless the JIT
removes the box, which it does not do at every call site. The fourteen
IBinaryInteger<T> implementations in the libraries now implement them
explicitly and forward to a helper that makes a constrained call to
TryWriteBigEndian or TryWriteLittleEndian and throws the same
exceptions as the default implementations.

Fix dotnet#77969

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Sep 17, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@tannergooding

Copy link
Copy Markdown
Member

This is a significant amount of code for a niche edge case API. You can even trivially work around by doing .AsSpan() to ensure you hit the span overload.

I'm not convinced it is worth the churn and additional implementation bloat nor that the box is meaningfully enough to warrant it.

@caraioniurie47

Copy link
Copy Markdown
Contributor Author

This is a significant amount of code for a niche edge case API. You can even trivially work around by doing .AsSpan() to ensure you hit the span overload.

I'm not convinced it is worth the churn and additional implementation bloat nor that the box is meaningfully enough to warrant it.

Thanks for looking. One data point on the workaround: in the PR description's benchmark, BigInteger.WriteBigEndian(buffer.AsSpan()) called through a non-inlined generic helper still allocates 32 B per call on main; the other span cases don't. If a change limited to BigInteger would be acceptable, I can trim it to that; otherwise I'm happy to close this. Which would be preferred?

@caraioniurie47

Copy link
Copy Markdown
Contributor Author

@tannergooding, following up on the question above with a re-check on a newer build.

On a main daily from 2026-09-30 (12.0.0-alpha.1.26480.102), .AsSpan() still doesn't avoid the box for BigInteger called through a non-inlined generic method. This prints 32 (bytes allocated by one call) on that build, and also on 11.0.0-rtm.26480.113 and 10.0.12:

using System.Numerics;
using System.Runtime.CompilerServices;

BigInteger value = BigInteger.Parse("123456789012345678901234567890");
byte[] buffer = new byte[64];

// Let tiered compilation promote Write to Tier1 first.
for (int round = 0; round < 50; round++)
{
    for (int i = 0; i < 100_000; i++)
    {
        Write(value, buffer.AsSpan());
    }
    Thread.Sleep(20);
}

long before = GC.GetAllocatedBytesForCurrentThread();
Write(value, buffer.AsSpan());
Console.WriteLine(GC.GetAllocatedBytesForCurrentThread() - before);

[MethodImpl(MethodImplOptions.NoInlining)]
static int Write<T>(T value, Span<byte> destination) where T : IBinaryInteger<T>
    => value.WriteBigEndian(destination);

In the Tier1 code the JIT inlines the default WriteBigEndian(Span<byte>) but not BigInteger's TryWriteBigEndian (IL size 250). The box is allocated and a reference into it is passed to that call as this. Through the same kind of helper on that build, WriteLittleEndian(Span<byte>) for int and Int128 and WriteBigEndian(Span<byte>) for long allocate nothing. In their Tier1 code TryWrite* is inlined and no box is allocated.

A BigInteger-only version is about 60 lines of product code, all in System.Runtime.Numerics, plus tests. Should I trim the PR to that, or close it?

Note

AI-generated, written at my direction and reviewed by me before posting. The snippet targets net10.0 and was run as shown on Windows x64, on 10.0.12 and on the daily builds 11.0.0-rtm.26480.113 and 12.0.0-alpha.1.26480.102 (installed with dotnet-install.ps1, run with DOTNET_ROLL_FORWARD=Major). The int, Int128 and long figures come from a separate program calling the same kind of [NoInlining] generic helper, run on 12.0.0-alpha.1.26480.102. The inlining statements come from DOTNET_JitDisasm listings of the Tier1 code on that build.

@tannergooding

Copy link
Copy Markdown
Member

I wasn't saying the box was going to go away. Rather, I was questioning whether the box is actually meaningful at all here given the work being done, the likely rarity of it all, etc.

It's a lot of additional API churn and surface for something that is unlikely to be problematic in practice.

@caraioniurie47

Copy link
Copy Markdown
Contributor Author

Understood, thanks for explaining. Closing this; since explicit implementations were tried and judged not worth the churn, #77969 could be closed as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Numerics community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Perf-trap with IBinaryInteger<T>.WriteLittleEndian

2 participants