-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Vectorize Convert.ToBase64String #71795
Conversation
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
src/libraries/System.Runtime.Extensions/tests/System/Convert.ToBase64String.cs
Show resolved
Hide resolved
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.
Thanks!
A bunch of related test failures |
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've done that before :-)
Is there anything still to do for Arm64 on this? I looked at the patch and couldn't see anything X64 specific in it. |
Initially, I submitted an x64-only implementation but then switched to the utf8 one + extension to utf16, so don't worry 🙂 |
Ok, that's fine then. |
Improvements on Linux-x64 dotnet/perf-autofiling-issues#6731 |
@@ -2336,7 +2339,30 @@ public static string ToBase64String(ReadOnlySpan<byte> bytes, Base64FormattingOp | |||
} | |||
|
|||
bool insertLineBreaks = (options == Base64FormattingOptions.InsertLineBreaks); | |||
string result = string.FastAllocateString(ToBase64_CalculateAndValidateOutputLength(bytes.Length, insertLineBreaks)); | |||
int outputLength = ToBase64_CalculateAndValidateOutputLength(bytes.Length, insertLineBreaks); |
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.
@EgorBo, do we need something similar to this change for Convert.ToBase64CharArray?
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.
@stephentoub makes sense!
x86 improvements dotnet/perf-autofiling-issues#6952 |
Arm64 improvements: dotnet/perf-autofiling-issues#6983, dotnet/perf-autofiling-issues#6977 |
ubuntu x64 improvements: dotnet/perf-autofiling-issues#6960 |
Use Utf8's version which is already vectorized for both x86 and arm and covert it to UTF16. Up to 3.5x faster for large inputs.
Benchmarks: