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

eliminate ActorPath.ToSerializationFormat UID allocations #6195

Merged
merged 13 commits into from
Nov 29, 2022
52 changes: 42 additions & 10 deletions src/core/Akka/Actor/ActorPath.cs
Original file line number Diff line number Diff line change
Expand Up @@ -557,8 +557,15 @@ public static bool TryParseParts(ReadOnlySpan<char> path, out ReadOnlySpan<char>
/// </summary>
/// <param name="prefix">the address or empty</param>
/// <returns> System.String. </returns>
private string Join(ReadOnlySpan<char> prefix)
private string Join(ReadOnlySpan<char> prefix, long? uid = null)
{
void AppendUidSpan(ref Span<char> writeable, int startPos, int sizeHint)
{
if (uid == null) return;
writeable[startPos] = '#';
SpanHacks.TryFormat(uid.Value, startPos+1, ref writeable, sizeHint);
}

if (_depth == 0)
{
Span<char> buffer = prefix.Length < 1024 ? stackalloc char[prefix.Length + 1] : new char[prefix.Length + 1];
Expand All @@ -576,17 +583,28 @@ private string Join(ReadOnlySpan<char> prefix)
totalLength += p._name.Length + 1;
p = p._parent;
}

// UID calculation
var uidSizeHint = 0;
if (uid != null)
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 whether it makes more sense to do something where we can avoid the branch in AppendUidSpan.

Ideas that come to mind:

  • Pad the allocated span by uint maxvalue+1 (21), that would let us just do the padding call within the if block.
  • Move this alloc stuff into a function that returns the span of proper length, either with UID filled or unfilled (and do the UID calc there)

{
// 1 extra character for the '#'
uidSizeHint = SpanHacks.Int64SizeInCharacters(uid.Value) + 1;
totalLength += uidSizeHint;
}

// Concatenate segments (in reverse order) into buffer with '/' prefixes
Span<char> buffer = totalLength < 1024 ? stackalloc char[totalLength] : new char[totalLength];
prefix.CopyTo(buffer);

var offset = buffer.Length;
ReadOnlySpan<char> name;
var offset = buffer.Length - uidSizeHint;
// append UID span first
AppendUidSpan(ref buffer, offset, uidSizeHint-1); // -1 for the '#'

p = this;
while (p._depth > 0)
{
name = p._name.AsSpan();
var name = p._name.AsSpan();
Aaronontheweb marked this conversation as resolved.
Show resolved Hide resolved
offset -= name.Length + 1;
buffer[offset] = '/';
name.CopyTo(buffer.Slice(offset + 1, name.Length));
Expand Down Expand Up @@ -676,7 +694,12 @@ public override bool Equals(object obj)
/// <returns> System.String. </returns>
public string ToStringWithAddress()
{
return ToStringWithAddress(_address);
return ToStringWithAddress(_address, false);
}

private string ToStringWithAddress(bool includeUid)
{
return ToStringWithAddress(_address, includeUid);
}

/// <summary>
Expand All @@ -685,7 +708,7 @@ public string ToStringWithAddress()
/// <returns>TBD</returns>
public string ToSerializationFormat()
{
return AppendUidFragment(ToStringWithAddress());
return ToStringWithAddress(true);
}

/// <summary>
Expand All @@ -700,8 +723,7 @@ public string ToSerializationFormatWithAddress(Address address)
// we never change address for IgnoreActorRef
return ToString();
}
var withAddress = ToStringWithAddress(address);
var result = AppendUidFragment(withAddress);
var result = ToStringWithAddress(address, true);
return result;
}

Expand All @@ -718,16 +740,26 @@ private string AppendUidFragment(string withAddress)
/// <param name="address"> The address. </param>
/// <returns> System.String. </returns>
public string ToStringWithAddress(Address address)
{
return ToStringWithAddress(address, false);
}

private string ToStringWithAddress(Address address, bool includeUid)
{
if (IgnoreActorRef.IsIgnoreRefPath(this))
{
// we never change address for IgnoreActorRef
return ToString();
}

long? uid = null;
if (includeUid && _uid != ActorCell.UndefinedUid)
uid = _uid;

if (_address.Host != null && _address.Port.HasValue)
return Join(_address.ToString().AsSpan());
return Join(_address.ToString().AsSpan(), uid);

return Join(address.ToString().AsSpan());
return Join(address.ToString().AsSpan(), uid);
}

/// <summary>
Expand Down
65 changes: 65 additions & 0 deletions src/core/Akka/Util/SpanHacks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,71 @@ public static int Parse(ReadOnlySpan<char> str)
throw new FormatException($"[{str.ToString()}] is now a valid numeric format");
}

private const char Negative = '-';
private static readonly char[] Numbers = { '0','1','2','3','4','5','6','7','8','9' };

/// <summary>
/// Can replace with int64.TryFormat in later versions of .NET.
/// </summary>
/// <param name="i">The integer we want to format into a string.</param>
/// <param name="startPos">Starting position in the destination span we're going to write from</param>
/// <param name="span">The span we're going to write our characters into.</param>
/// <param name="sizeHint">Optional size hint, in order to avoid recalculating it.</param>
/// <returns></returns>
public static int TryFormat(long i, int startPos, ref Span<char> span, int sizeHint = 0)
{
var index = 0;
var negative = i < 0;
if (i == 0)
{
span[index++] = Numbers[0];
return index;
}

var targetLength = sizeHint > 0 ? sizeHint : Int64SizeInCharacters(i);
Aaronontheweb marked this conversation as resolved.
Show resolved Hide resolved
if(negative){
i = Math.Abs(i);
}

while (i > 0)
{
span[startPos + targetLength - index++ - 1] = Numbers[i % 10];
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason we need startPos here? i.e. would it be better to have callers .Slice() the input span to the correct starting position?

Copy link
Member

Choose a reason for hiding this comment

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

Additional Question: would it be better to do = (i%10)+'0' here? I guess it depends on whether you'd rather throw (slower but potentially safer) or overflow (faster but potentially unsafer.)

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'll add a dedicated benchmark for this function so we can measure it

Copy link
Member Author

Choose a reason for hiding this comment

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

Is there a reason we need startPos here? i.e. would it be better to have callers .Slice() the input span to the correct starting position?

I don't think it hurts anything - I just didn't wanted all changes to be made on the same input Span

i /= 10;
}

if(negative){
span[0] = Negative;
index++;
}

return index;
}

/// <summary>
/// How many characters do we need to represent this int as a string?
/// </summary>
/// <param name="i">The int.</param>
/// <returns>Character length.</returns>
public static int Int64SizeInCharacters(long i)
{
// still need 1 char to represent '0'
if(i == 0) return 1;

// account for negative characters
var startLen = i < 0 ? 1 : 0;

i = Math.Abs(i);

// count sig figs
while (i > 0)
Copy link
Member

@to11mtm to11mtm Oct 18, 2022

Choose a reason for hiding this comment

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

Would an if or Jump based table based on length potentially be better here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Might very well be - I haven't benchmarked these methods themselves, only in aggregate

Copy link
Member

Choose a reason for hiding this comment

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

So, looking at .NET, one thing that we -can- do here, is something like:

while (i > 100)
{
   i = i / 100;
   startLen +=2;
}
if (i > 10)
{
   startLen++;
}
return startLen +1; //Could alternatively 'preload' the first character in the negative ternary.

The advantage is that division happens half as often, even if the ASM -may- be a little bit larger... but we can elide elsewhere. :)

{
i = i / 10;
startLen++;
}

return startLen;
}

/// <summary>
/// Parses an integer from a string.
/// </summary>
Expand Down