-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix perf regressions in Utf8Formatter for integers #85277
Conversation
When I added UTF8 support to the core numeric types, I also just routed Utf8Formatter to use the public TryFormat API on each type. That, however, regressed some microbenchmarks due to a) going from `StandardFormat` to a `ReadOnlySpan<char>` format and then parsing it back out and b) removing some of the inlining that was there previously. This change puts back into Utf8Formatter.TryFormat the handling of the format and then delegating to the relevant helpers that already exist rather than always going through the public entrypoint (it doesn't do so for 'n', but that's also much rarer to use on a hot path and is also in general more expensive).
Tagging subscribers to this area: @dotnet/area-system-memory Issue DetailsWhen I added UTF8 support to the core numeric types, I also just routed Utf8Formatter to use the public TryFormat API on each type. That, however, regressed some microbenchmarks due to a) going from private byte[] _buffer = new byte[100];
[Benchmark]
[Arguments(42)]
public bool TryFormatInt32_Default(int value) => Utf8Formatter.TryFormat(value, _buffer, out _);
[Benchmark]
[Arguments(123L)]
public bool TryFormatInt64_Default(long value) => Utf8Formatter.TryFormat(value, _buffer, out _);
[Benchmark]
[Arguments(12345UL)]
public bool TryFormatUInt64_Default(ulong value) => Utf8Formatter.TryFormat(value, _buffer, out _);
[Benchmark]
[Arguments(42)]
public bool TryFormatInt32_D2(int value) => Utf8Formatter.TryFormat(value, _buffer, out _, new StandardFormat('D', 2));
[Benchmark]
[Arguments(0x1234UL)]
public bool TryFormatUInt64_X4(ulong value) => Utf8Formatter.TryFormat(value, _buffer, out _, new StandardFormat('X', 4));
[Benchmark]
[Arguments(long.MinValue)]
public bool TryFormatInt64_X(long value) => Utf8Formatter.TryFormat(value, _buffer, out _, new StandardFormat('X'));
|
...raries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Formatter/Utf8Formatter.Integer.cs
Show resolved
Hide resolved
Still seeing a regression in this test https://pvscmdupload.blob.core.windows.net/reports/allTestHistory/refs/heads/main_x64_ubuntu%2018.04/System.Buffers.Text.Tests.Utf8FormatterTests.FormatterDecimal(value%3a%20123456.789).html Found during 7.0 -> 8.0 perf comparison. |
When I added UTF8 support to the core numeric types, I also just routed Utf8Formatter to use the public TryFormat API on each type. That, however, regressed some microbenchmarks due to a) going from
StandardFormat
to aReadOnlySpan<char>
format and then parsing it back out and b) removing some of the inlining that was there previously. This change puts back into Utf8Formatter.TryFormat the handling of the format and then delegating to the relevant helpers that already exist rather than always going through the public entrypoint (it doesn't do so for 'n', but that's also much rarer to use on a hot path and is also in general more expensive).