Skip to content
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

Removed StringBuilder allocation from Base64Encoding #5051

Merged
merged 3 commits into from May 29, 2021

Conversation

Aaronontheweb
Copy link
Member

Before (ActorSelection + Ask)

BenchmarkDotNet=v0.13.0, OS=Windows 10.0.19041.985 (2004/May2020Update/20H1)
AMD Ryzen 7 1700, 1 CPU, 16 logical and 8 physical cores
.NET SDK=5.0.203
  [Host]     : .NET Core 3.1.15 (CoreCLR 4.700.21.21202, CoreFX 4.700.21.21402), X64 RyuJIT
  DefaultJob : .NET Core 3.1.15 (CoreCLR 4.700.21.21202, CoreFX 4.700.21.21402), X64 RyuJIT

Method Iterations Mean Error StdDev Gen 0 Gen 1 Gen 2 Allocated
RequestResponseActorSelection 10000 100.723 ms 2.0080 ms 5.6308 ms 5000.0000 - - 20 MB
CreateActorSelection 10000 7.564 ms 0.2305 ms 0.6795 ms 953.1250 - - 4 MB

After (ActorSelection + Ask)

BenchmarkDotNet=v0.13.0, OS=Windows 10.0.19041.985 (2004/May2020Update/20H1)
AMD Ryzen 7 1700, 1 CPU, 16 logical and 8 physical cores
.NET SDK=5.0.203
  [Host]     : .NET Core 3.1.15 (CoreCLR 4.700.21.21202, CoreFX 4.700.21.21402), X64 RyuJIT
  DefaultJob : .NET Core 3.1.15 (CoreCLR 4.700.21.21202, CoreFX 4.700.21.21402), X64 RyuJIT

Method Iterations Mean Error StdDev Gen 0 Gen 1 Gen 2 Allocated
RequestResponseActorSelection 10000 89.490 ms 1.2584 ms 0.9824 ms 4666.6667 - - 19 MB
CreateActorSelection 10000 5.445 ms 0.0385 ms 0.0342 ms 953.1250 - - 4 MB


public static string Base64Encode(this long value, string prefix)
{
Span<char> sb = stackalloc char[11 + prefix?.Length ?? 0];
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing program is here:

https://gist.github.com/Aaronontheweb/449d565754d5f9bce475653842317277

11 is the number of characters it takes to represent long.MaxValue in base64 encoded strings

Copy link
Member

@to11mtm to11mtm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple little things.

src/core/Akka/Util/Base64Encoding.cs Outdated Show resolved Hide resolved
src/core/Akka/Util/Base64Encoding.cs Outdated Show resolved Hide resolved
@Aaronontheweb Aaronontheweb enabled auto-merge (squash) May 29, 2021 16:43

/// <summary>
/// TBD
/// </summary>
/// <param name="value">TBD</param>
/// <param name="sb">TBD</param>
/// <returns>TBD</returns>
[Obsolete("Do not use. Pass in prefix as a string instead.")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we change this comment, since our method that takes string is internal now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still useful since this method really shouldn't have been exposed as public anyway, IMHO

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We had a bad habit in the very early days of Akka.NET making a lot of the internals public "in case someone wanted to play with them!" and we've been gradually undoing that in our minor version releases....

do
{
var index = (int)(next & 63);
sb[spanIndex++] = Base64Chars[index];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder, is there any advantage at this point to changing Base64Chars to ReadOnlyMemory?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so since it's basically just integer accessors

@Aaronontheweb Aaronontheweb merged commit 566dbe3 into akkadotnet:dev May 29, 2021
@Aaronontheweb Aaronontheweb deleted the perf/remove-SB-base64 branch May 29, 2021 19:42
This was referenced Jun 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants