-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Improve {u}int/long.ToString/TryFormat throughput by pre-computing the length #17432
Conversation
Currently we create a temporary buffer on the stack, format into it, and then copy from that stack buffer into either the target span (for TryFormat) or into a new string (for ToString. Following the approach as (and sharing the same code from) Utf8Formatter, where it first counts the number of digits in the output in order to determine an exact length, this commit changes the implementation to skip the temporary buffer and just format directly into the destination span or string. This results in a very measurable performance boost.
Unrelated, but wondering if some of the pointer arithmetic data dependency could be broken in some of these loops? Like in int i = (int)(buffer + Int32Precision - p);
number.scale = i;
char* dst = number.digits;
while (--i >= 0)
*dst++ = *p++; There's a result dependency on So could change to only depending on the result of int count = (int)(buffer + Int32Precision - p);
number.scale = count;
char* dst = number.digits;
for (int i = 0; i < count; i++)
{
// *(dst + i) = *(p + i);
dst[i] = p[i];
} Not sure if the above is completely correct; easily confused by postfix vs prefix operators, and that uses both! |
@dotnet-bot test Ubuntu arm Cross Checked Innerloop Build and Test please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Improve {u}int/long.ToString/TryFormat throughput by pre-computing the length Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
Improve {u}int/long.ToString/TryFormat throughput by pre-computing the length Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
@@ -52,6 +52,7 @@ | |||
<Compile Include="$(MSBuildThisFileDirectory)System\Buffers\MemoryManager.cs" /> | |||
<Compile Include="$(MSBuildThisFileDirectory)System\Buffers\TlsOverPerCoreLockedStacksArrayPool.cs" /> | |||
<Compile Include="$(MSBuildThisFileDirectory)System\Buffers\Utilities.cs" /> | |||
<Compile Include="$(MSBuildThisFileDirectory)System\Buffers\Text\FormattingHelpers.CountDigits.cs" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: sort order
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's wrong with the sort order? Don't we normally put files in a directory before folders in that directory?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't know that was what we were doing.
Looking at this file, we seem to be following alphabetical order (only):
<Compile Include="$(MSBuildThisFileDirectory)System\Globalization\UnicodeCategory.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Guid.cs" />
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW VS inserts items with simple string sort.
Of course it won't ever insert into an imported file like this.
Awesome! Perf improvement across the board :) |
@Anipik, is the mirror to corefx running? I haven't seen the relevant pieces here mirrored yet. Thanks. |
Improve {u}int/long.ToString/TryFormat throughput by pre-computing the length Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com>
Done started the mirror |
Thanks |
Improve {u}int/long.ToString/TryFormat throughput by pre-computing the length Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com>
The first commit just moves the Count{Hex}Digits methods from https://github.com/dotnet/corefx/blob/master/src/System.Memory/src/System/Buffers/Text/Utf8Formatter/FormattingHelpers.cs into a partial FormattingHelpers.CountDigits.cs file in the shared partition. Once those changes replicate to corefx, I'll dedup the code there.
The second commit then uses Count{Hex}Digits in the ToString and TryFormat methods of int, uint, long, and ulong, in particular for the default D format (and some G configurations) as well as the X format. Currently we create a temporary buffer on the stack, format into it, and then copy from that stack buffer into either the target span (for TryFormat) or into a new string (for ToString. Following the approach (and sharing the same code) from Utf8Formatter, where it first counts the number of digits in the output in order to determine an exact length, this commit changes the implementation to skip the temporary buffer and just format directly into the destination span or string.
Contributes to https://github.com/dotnet/coreclr/issues/15364
cc: @jkotas, @ahsonkhan, @danmosemsft