Remove `Uri.TryParse` and replace with `Span<char>` parsing #5039

Aaronontheweb · 2021-05-26T02:06:52Z

Part of #5030 - largely an experiment

Aaronontheweb · 2021-05-26T02:28:05Z

`ActorPath.Parse` Before

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	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
ActorPath_Parse	1,672.00 ns	9.184 ns	7.669 ns	0.2136	-	-	896 B
ActorPath_Concat	54.91 ns	1.128 ns	1.299 ns	0.0268	-	-	112 B
ActorPath_Equals	26.08 ns	0.095 ns	0.085 ns	-	-	-	-
ActorPath_ToString	119.24 ns	0.835 ns	0.740 ns	0.0210	-	-	88 B

`ActorPath.Parse` After

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	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
ActorPath_Parse	517.56 ns	7.238 ns	5.651 ns	0.1392	-	-	584 B
ActorPath_Concat	63.75 ns	1.323 ns	1.173 ns	0.0267	-	-	112 B
ActorPath_Equals	20.58 ns	0.433 ns	0.464 ns	-	-	-	-
ActorPath_ToString	212.42 ns	2.703 ns	2.528 ns	0.0515	-	-	216 B
ActorPath_ToSerializationFormat	234.79 ns	2.901 ns	2.265 ns	0.0515	-	-	216 B
ActorPath_ToSerializationFormatWithAddress	245.78 ns	4.985 ns	6.122 ns	0.0515	-	-	216 B

to11mtm · 2021-05-26T03:41:43Z

src/core/Akka/Actor/ActorPath.cs

-            {
-                // Protocol must start with 'akka.*
+
+            var fullScheme = spanified.Slice(0, firstColonPos).ToString().ToLowerInvariant();


Stupid questions, can we use stackalloc here to avoid allocating two strings?

yeah there should be a way to do the ToLower part without allocating a second string

Aaronontheweb · 2021-05-26T04:05:30Z

FsCheck.Xunit.PropertyFailedException : 
Falsifiable, after 1 test (0 shrinks) (StdGen (795753292, 296894447)):
Original:
[::1]:1337

---- System.UriFormatException : Can not parse an ActorPath: akka.tcp://foo@[::1]:1337/user/foo

Ah, forgot to test IPV6 and IPV4 addresses explicitly

Aaronontheweb · 2021-05-27T14:53:17Z

`RemotePingPong` Benchmarks

Before

OSVersion:                         Microsoft Windows NT 6.2.9200.0
ProcessorCount:                    16
ClockSpeed:                        0 MHZ
Actor Count:                       32
Messages sent/received per client: 200000  (2e5)
Is Server GC:                      True
Thread count:                      109

Num clients, Total [msg], Msgs/sec, Total [ms]
         1,  200000,     85107,    2350.02
         5, 1000000,    169924,    5885.53
        10, 2000000,    170489,   11731.02
        15, 3000000,    172444,   17397.85
        20, 4000000,    171726,   23293.97
        25, 5000000,    176773,   28285.99
        30, 6000000,    176409,   34012.59

After

OSVersion:                         Microsoft Windows NT 6.2.9200.0
ProcessorCount:                    16
ClockSpeed:                        0 MHZ
Actor Count:                       32
Messages sent/received per client: 200000  (2e5)
Is Server GC:                      True
Thread count:                      109

Num clients, Total [msg], Msgs/sec, Total [ms]
         1,  200000,    107067,    1868.05
         5, 1000000,    181984,    5495.15
        10, 2000000,    182117,   10982.94
        15, 3000000,    183151,   16380.79
        20, 4000000,    182357,   21935.98
        25, 5000000,    180643,   27679.77
        30, 6000000,    179587,   33410.05

Edit: whoops, my previous comment about the LruBoundedCache was wrong. We recreate the ActorSystems during each benchmark, so the impact of ActorPath.Parse is really felt during each stage - but it's easier to miss its impact in the higher call count benchmarks because it gets averaged in with all of the other operations that depend upon caching.

Aaronontheweb · 2021-05-27T15:13:25Z

Final before / after numbers, starting from dev and leading to here:

`ActorPath` Before

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	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
ActorPath_Parse	1,938.91 ns	5.334 ns	4.454 ns	0.2670	-	-	1,128 B
ActorPath_Concat	59.56 ns	0.876 ns	0.819 ns	0.0421	-	-	176 B
ActorPath_Equals	24.97 ns	0.090 ns	0.080 ns	-	-	-	-
ActorPath_ToString	125.80 ns	1.200 ns	1.122 ns	0.0210	-	-	88 B

`ActorPath` After

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	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
ActorPath_Parse	558.55 ns	9.212 ns	8.617 ns	0.1392	-	-	584 B
ActorPath_Concat	57.82 ns	1.232 ns	3.022 ns	0.0267	-	-	112 B
ActorPath_Equals	19.11 ns	0.199 ns	0.176 ns	-	-	-	-
ActorPath_ToString	203.28 ns	2.391 ns	2.236 ns	0.0515	-	-	216 B
ActorPath_ToSerializationFormat	218.22 ns	3.196 ns	2.833 ns	0.0515	-	-	216 B
ActorPath_ToSerializationFormatWithAddress	237.46 ns	4.637 ns	6.029 ns	0.0515	-	-	216 B

Lost a bit of performance as a result of implementing proper IPV6 checking, but that was necessary.

Arkatufus

Looks good

Aaronontheweb · 2021-05-27T15:14:53Z

Interesting

ActorPath_ToString	125.80 ns	1.200 ns	1.122 ns	0.0210	-	-	88 B

That value actually increased with this PR. I'm working on optimizing that next, but I'll have to look into why.

Aaronontheweb · 2021-05-27T15:15:33Z

Interesting

ActorPath_ToString 125.80 ns 1.200 ns 1.122 ns 0.0210 - - 88 B
That value actually increased with this PR. I'm working on optimizing that next, but I'll have to look into why.

I wonder if it's because we're already including the UID fragment in our final output now....

Aaronontheweb · 2021-05-27T15:37:45Z

LOL it's because I made the ActorPath.ToString() benchmark use a more complicated path. Nevermind.

to11mtm

Found some edge cases, might be more but wanted to throw at least these out.

to11mtm · 2021-05-27T19:54:48Z

src/core/Akka/Actor/ActorPath.cs

+            var openBracket = spanified.IndexOf('[');
+            var closeBracket = spanified.IndexOf(']');
+            if (openBracket > -1 && closeBracket > openBracket)


Do we need to have any additional guarding around [] being contained in a path, i.e. possibly misdetecting IPV6 instead of IPV4 here, and then accidentally failing the parse?

One thought is we could pre-check for the slash in the path, and then math based off that index... Would then have to get the slash again later or track it's position as we trim, but would avoid this case if we need to worry about it.

I believe the [ and ] characters are illegal for actor names, so that shouldn't be a problem.

akka.net/src/core/Akka/Actor/ActorPath.cs

Lines 109 to 131 in c663bc2

internal static readonly char[] ValidSymbols = @"""-_.*$+:@&=,!~';""()".ToCharArray();

/// <summary>

/// Method that checks if actor name conforms to RFC 2396, http://www.ietf.org/rfc/rfc2396.txt

/// Note that AKKA JVM does not allow parenthesis ( ) but, according to RFC 2396 those are allowed, and

/// since we use URL Encode to create valid actor names, we must allow them.

/// </summary>

/// <param name="s">TBD</param>

/// <returns>TBD</returns>

public static bool IsValidPathElement(string s)

{

if (IsNullOrEmpty(s))

{

return false;

}

return !s.StartsWith("$") && Validate(s);

}

private static bool IsValidChar(char c) => (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') ||

(c >= '0' && c <= '9') || ValidSymbols.Contains(c);

private static bool IsHexChar(char c) => (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F') ||

(c >= '0' && c <= '9');

One thought is we could pre-check for the slash in the path, and then math based off that index... Would then have to get the slash again later or track it's position as we trim, but would avoid this case if we need to worry about it.

I think this is a good idea - won't hurt performance much, because if the check fails we stop the parse and return false anyway. If we are successful we have to retrieve this value anyway in the future.

to11mtm · 2021-05-27T20:18:25Z

src/core/Akka/Actor/ActorPath.cs

@@ -327,74 +327,165 @@ public static bool TryParse(string path, out ActorPath actorPath)
        {
            actorPath = null;

+            if (!TryParseAddress(path, out var address, out var absoluteUri)) return false;


NIT: would this technically be a relative URI?

You can have a relative ActorSelection but not a relative ActorPath - we never allowed parsing relative ones before either:

akka.net/src/core/Akka/Actor/ActorPath.cs

Lines 326 to 342 in c663bc2

public static bool TryParse(string path, out ActorPath actorPath)

{

actorPath = null;

Address address;

Uri uri;

if (!TryParseAddress(path, out address, out uri)) return false;

var pathElements = uri.AbsolutePath.Split('/');

actorPath = new RootActorPath(address) / pathElements.Skip(1);

if (uri.Fragment.StartsWith("#"))

{

var uid = int.Parse(uri.Fragment.Substring(1));

actorPath = actorPath.WithUid(uid);

}

return true;

}

This is because in order for the path to compute things like its own string, it has to be able to work its way up the food chain.

to11mtm · 2021-05-27T20:23:56Z

src/core/Akka/Actor/ActorPath.cs

+            if (!fullScheme.StartsWith("akka"))
+                return false;
+
+            spanified = spanified.Slice(firstColonPos + 1);
+            if (!(spanified[0] == '/' && spanified[1] == '/'))


Could throw here on a malformed URI on either of these last two lines; may want to consider:

For the Slice:

Maybe a length check with the scheme check? if (!fullScheme.StartsWith("akka") || spanified.Length < firstColonPos + 2) (Double check my math here but I think you get the idea)

For the If statement:

if (spanified.Length<2 || !(spanified[0] == '/' && spanified[1] == '/'))

Got it - I'll add that

Abstraction of ServiceProvider, Improving Akka.DependencyInjection (akkadotnet#4814) * Abstraction of ServiceProvider * introduced non-breaking Akka.DependencyInjection API changes * fixed unit tests / Props bug * fixed up DelegateInjectionSpecs * Added type checking for `Props(Type type, params object[] args)` * fixed non-generic `Props()` method Co-authored-by: Aaron Stannard <aaron@petabridge.com> completed work on ActorPath parsing

Aaronontheweb · 2021-05-28T00:54:56Z

Final numbers - mostly from safely adding the UID back at the end. We were just string-fying that directly into the actor path before. Maybe that's ok?

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	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
ActorPath_Parse	672.90 ns	13.111 ns	13.464 ns	0.1392	-	-	584 B
ActorPath_Concat	77.02 ns	1.583 ns	1.885 ns	0.0267	-	-	112 B
ActorPath_Equals	24.41 ns	0.495 ns	0.509 ns	-	-	-	-
ActorPath_ToString	218.45 ns	4.101 ns	7.075 ns	0.0515	-	-	216 B
ActorPath_ToSerializationFormat	245.74 ns	4.618 ns	5.841 ns	0.0515	-	-	216 B
ActorPath_ToSerializationFormatWithAddress	253.03 ns	4.753 ns	9.381 ns	0.0515	-	-	216 B

Aaronontheweb · 2021-05-28T01:22:14Z

Was able to improve the final WithUid parse inside the span processing loop; restored some throughput that way.

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	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
ActorPath_Parse	580.69 ns	11.569 ns	13.772 ns	0.1392	-	-	584 B
ActorPath_Concat	59.04 ns	1.219 ns	1.628 ns	0.0267	-	-	112 B
ActorPath_Equals	19.42 ns	0.298 ns	0.264 ns	-	-	-	-
ActorPath_ToString	205.58 ns	2.330 ns	2.180 ns	0.0401	-	-	168 B
ActorPath_ToSerializationFormat	227.92 ns	2.169 ns	1.694 ns	0.0401	-	-	168 B
ActorPath_ToSerializationFormatWithAddress	250.58 ns	4.903 ns	12.917 ns	0.0401	-	-	168 B

Aaronontheweb · 2021-05-28T01:23:44Z

src/core/Akka/Actor/ActorPath.cs

                p = p.Parent;
            }

-            return new string(buffer);
+            return buffer.ToString();


So this method prevents an additional copy of the string buffer from being made, which is why Gen 0 GC is lower for this iteration.

Aaronontheweb added akka-actor perf labels May 26, 2021

to11mtm reviewed May 26, 2021

View reviewed changes

Aaronontheweb mentioned this pull request May 27, 2021

Span<char>-ifying ActorPath and Address parsing #5030

Closed

Arkatufus approved these changes May 27, 2021

View reviewed changes

Aaronontheweb enabled auto-merge (squash) May 27, 2021 15:37

to11mtm reviewed May 27, 2021

View reviewed changes

Aaronontheweb force-pushed the perf/Address-parse branch from f3c2cd4 to 6b2240e Compare May 28, 2021 00:47

Merge branch 'dev' into perf/Address-parse

fce511b

restore Parse performance to ActorPath.Parse

6a46519

Aaronontheweb commented May 28, 2021

View reviewed changes

Aaronontheweb added 2 commits May 27, 2021 21:12

fixed ActorPath.ToString()

371b2b0

Merge branch 'dev' into perf/Address-parse

328d21e

Aaronontheweb disabled auto-merge May 28, 2021 04:54

Aaronontheweb merged commit c21289e into akkadotnet:dev May 28, 2021

Aaronontheweb deleted the perf/Address-parse branch May 28, 2021 04:55

Aaronontheweb mentioned this pull request Jun 3, 2021

v1.4.21-beta1 Release Notes #5071

Merged

This was referenced Jun 16, 2021

v1.4.21 final release notes #5095

Merged

v1.4.21 Release #5096

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove `Uri.TryParse` and replace with `Span<char>` parsing #5039

Remove `Uri.TryParse` and replace with `Span<char>` parsing #5039

Aaronontheweb commented May 26, 2021

Aaronontheweb commented May 26, 2021

to11mtm May 26, 2021

Aaronontheweb May 26, 2021

Aaronontheweb commented May 26, 2021

Aaronontheweb commented May 27, 2021 •

edited

Aaronontheweb commented May 27, 2021

Arkatufus left a comment

Aaronontheweb commented May 27, 2021

Aaronontheweb commented May 27, 2021

Aaronontheweb commented May 27, 2021

to11mtm left a comment

to11mtm May 27, 2021

Aaronontheweb May 27, 2021

Aaronontheweb May 27, 2021

to11mtm May 27, 2021

Aaronontheweb May 27, 2021

to11mtm May 27, 2021

Aaronontheweb May 27, 2021

Aaronontheweb commented May 28, 2021

Aaronontheweb commented May 28, 2021

Aaronontheweb May 28, 2021

	internal static readonly char[] ValidSymbols = @"""-_.*$+:@&=,!~';""()".ToCharArray();

	/// <summary>
	/// Method that checks if actor name conforms to RFC 2396, http://www.ietf.org/rfc/rfc2396.txt
	/// Note that AKKA JVM does not allow parenthesis ( ) but, according to RFC 2396 those are allowed, and
	/// since we use URL Encode to create valid actor names, we must allow them.
	/// </summary>
	/// <param name="s">TBD</param>
	/// <returns>TBD</returns>
	public static bool IsValidPathElement(string s)
	{
	if (IsNullOrEmpty(s))
	{
	return false;
	}
	return !s.StartsWith("$") && Validate(s);
	}

	private static bool IsValidChar(char c) => (c >= 'a' && c <= 'z') \|\| (c >= 'A' && c <= 'Z') \|\|
	(c >= '0' && c <= '9') \|\| ValidSymbols.Contains(c);

	private static bool IsHexChar(char c) => (c >= 'a' && c <= 'f') \|\| (c >= 'A' && c <= 'F') \|\|
	(c >= '0' && c <= '9');

	public static bool TryParse(string path, out ActorPath actorPath)
	{
	actorPath = null;


	Address address;
	Uri uri;
	if (!TryParseAddress(path, out address, out uri)) return false;
	var pathElements = uri.AbsolutePath.Split('/');
	actorPath = new RootActorPath(address) / pathElements.Skip(1);
	if (uri.Fragment.StartsWith("#"))
	{
	var uid = int.Parse(uri.Fragment.Substring(1));
	actorPath = actorPath.WithUid(uid);
	}
	return true;
	}

Remove Uri.TryParse and replace with Span<char> parsing #5039

Remove Uri.TryParse and replace with Span<char> parsing #5039

Conversation

Aaronontheweb commented May 26, 2021

Aaronontheweb commented May 26, 2021

ActorPath.Parse Before

ActorPath.Parse After

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aaronontheweb commented May 26, 2021

Aaronontheweb commented May 27, 2021 • edited

RemotePingPong Benchmarks

Before

After

Aaronontheweb commented May 27, 2021

ActorPath Before

ActorPath After

Arkatufus left a comment

Choose a reason for hiding this comment

Aaronontheweb commented May 27, 2021

Aaronontheweb commented May 27, 2021

Aaronontheweb commented May 27, 2021

to11mtm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aaronontheweb commented May 28, 2021

Aaronontheweb commented May 28, 2021

Choose a reason for hiding this comment

Remove `Uri.TryParse` and replace with `Span<char>` parsing #5039

Remove `Uri.TryParse` and replace with `Span<char>` parsing #5039

`ActorPath.Parse` Before

`ActorPath.Parse` After

Aaronontheweb commented May 27, 2021 •

edited

`RemotePingPong` Benchmarks

`ActorPath` Before

`ActorPath` After