-
Notifications
You must be signed in to change notification settings - Fork 241
Reduce allocations in ChecksumUtilities.BytesToString #12359
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,8 +3,10 @@ | |
|
|
||
| using System; | ||
| using System.Collections.Immutable; | ||
| using System.Globalization; | ||
| using Microsoft.AspNetCore.Razor.PooledObjects; | ||
|
|
||
| #if NET9_0_OR_GREATER | ||
| using System.Runtime.InteropServices; | ||
| #endif | ||
|
|
||
| namespace Microsoft.AspNetCore.Razor.Language; | ||
|
|
||
|
|
@@ -17,15 +19,32 @@ public static string BytesToString(ImmutableArray<byte> bytes) | |
| throw new ArgumentNullException(nameof(bytes)); | ||
| } | ||
|
|
||
| using var _ = StringBuilderPool.GetPooledObject(out var builder); | ||
| builder.EnsureCapacity(bytes.Length); | ||
| #if NET9_0_OR_GREATER | ||
| var bytesArray = ImmutableCollectionsMarshal.AsArray(bytes)!; | ||
|
|
||
| return Convert.ToHexStringLower(bytesArray); | ||
| #else | ||
| const int StackAllocThreshold = 256; // reasonable for stackalloc | ||
| var charCount = bytes.Length * 2; | ||
|
|
||
| // As this should be getting called with a Checksum array of length 32, this shouldn't allocate | ||
| var buffer = charCount <= StackAllocThreshold | ||
| ? stackalloc char[charCount] | ||
| : new char[charCount]; | ||
|
|
||
| var bufferIndex = 0; | ||
| foreach (var b in bytes) | ||
| { | ||
| // The x2 format means lowercase hex, where each byte is a 2-character string. | ||
| builder.Append(b.ToString("x2", CultureInfo.InvariantCulture)); | ||
| // Write hex chars directly | ||
| buffer[bufferIndex++] = GetHexChar(b >> 4); | ||
| buffer[bufferIndex++] = GetHexChar(b & 0xF); | ||
| } | ||
|
|
||
| return builder.ToString(); | ||
| // Allocate the final string | ||
| return buffer.ToString(); | ||
|
ToddGrun marked this conversation as resolved.
Outdated
|
||
|
|
||
| static char GetHexChar(int value) | ||
| => (char)(value < 10 ? '0' + value : 'a' + (value - 10)); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this faster than indexing into a pre-allocated array of chars?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably slightly slower, but I didn't want to duplicate HexConverter.CharToHexLookup, especially as this won't be used OOP once we move to net9
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wouldn't duplicate all of that either! I was just thinking of something like this: private static char GetHexChar(int value) => s_hexChars[value];
private static readonly char[] s_hexChars = new[] { '0', '1', '2', '3', '4', '5', '6', '7, '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' };
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW, that's definitely into the micro-optimization level. I agree that it's not needed for just |
||
| #endif | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.