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

Improve performance on IActorRef.Child API #5242

Merged
merged 20 commits into from
Sep 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
9a50416
improveme child lookup performance
Aaronontheweb Sep 1, 2021
0aa7122
modified `IActorRef.Child` API to accept a `IReadOnlyList<T>` instead…
Aaronontheweb Sep 1, 2021
bf6e162
fixed a bug with low-level resolution
Aaronontheweb Sep 1, 2021
fdac3b9
approved core API changes
Aaronontheweb Sep 1, 2021
bc91ad8
approved again....
Aaronontheweb Sep 1, 2021
d754014
fixed - was missing one approval
Aaronontheweb Sep 1, 2021
3910d11
added extension method to simplify copying
Aaronontheweb Sep 1, 2021
f8f5342
cleaned up RepointableActorRef
Aaronontheweb Sep 1, 2021
3197d0c
cleaned up branching
Aaronontheweb Sep 1, 2021
6b590d7
implemented CopyTo method for debugging
Aaronontheweb Sep 2, 2021
8cce9b7
added child-resolve program
Aaronontheweb Sep 2, 2021
16e33c8
removed expensive allocation from `MinimalActorRef.GetChild`
Aaronontheweb Sep 2, 2021
702d7b8
Merge branch 'dev' into perf/improve-ResolveAPI
Aaronontheweb Sep 2, 2021
26db634
Merge branch 'perf/improve-ResolveAPI' of https://github.com/Aaronont…
Aaronontheweb Sep 2, 2021
bc66127
Revert "added child-resolve program"
Aaronontheweb Sep 2, 2021
075159b
Merge branch 'dev' into perf/improve-ResolveAPI
Aaronontheweb Sep 2, 2021
d04e5a7
Merge branch 'dev' into perf/improve-ResolveAPI
Aaronontheweb Sep 2, 2021
9f8b852
fixed indexer operation
Aaronontheweb Sep 2, 2021
83224be
simplified branching inside `TryGetSingleChild`
Aaronontheweb Sep 2, 2021
60b3537
removed redundant TryOut methods
Aaronontheweb Sep 2, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ namespace Akka.Actor
public System.Collections.Generic.IEnumerable<Akka.Actor.IInternalActorRef> GetChildren() { }
public static Akka.Actor.IActorRef GetCurrentSelfOrNoSender() { }
public static Akka.Actor.IActorRef GetCurrentSenderOrNoSender() { }
[System.ObsoleteAttribute("Use TryGetSingleChild [0.7.1]")]
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed TryGetSingleChild since no one was actually using its boolean functionality at all

public Akka.Actor.IInternalActorRef GetSingleChild(string name) { }
public void Init(bool sendSupervise, Akka.Dispatch.MailboxType mailboxType) { }
public Akka.Actor.Internal.ChildRestartStats InitChild(Akka.Actor.IInternalActorRef actor) { }
Expand Down Expand Up @@ -127,7 +126,6 @@ namespace Akka.Actor
public void TerminatedQueuedFor(Akka.Actor.IActorRef subject, Akka.Util.Option<object> customMessage) { }
public bool TryGetChildStatsByName(string name, out Akka.Actor.Internal.IChildStats child) { }
protected bool TryGetChildStatsByRef(Akka.Actor.IActorRef actor, out Akka.Actor.Internal.ChildRestartStats child) { }
public bool TryGetSingleChild(string name, out Akka.Actor.IInternalActorRef child) { }
public void UnbecomeStacked() { }
protected void UnreserveChild(string name) { }
public Akka.Actor.IActorRef Unwatch(Akka.Actor.IActorRef subject) { }
Expand Down Expand Up @@ -1081,7 +1079,7 @@ namespace Akka.Actor
bool IsTerminated { get; }
Akka.Actor.IInternalActorRef Parent { get; }
Akka.Actor.IActorRefProvider Provider { get; }
Akka.Actor.IActorRef GetChild(System.Collections.Generic.IEnumerable<string> name);
Akka.Actor.IActorRef GetChild(System.Collections.Generic.IReadOnlyList<string> name);
void Restart(System.Exception cause);
void Resume(System.Exception causedByFailure = null);
[System.ObsoleteAttribute("Use SendSystemMessage(message) [1.1.0]")]
Expand Down Expand Up @@ -1202,7 +1200,7 @@ namespace Akka.Actor
public abstract bool IsTerminated { get; }
public abstract Akka.Actor.IInternalActorRef Parent { get; }
public abstract Akka.Actor.IActorRefProvider Provider { get; }
public abstract Akka.Actor.IActorRef GetChild(System.Collections.Generic.IEnumerable<string> name);
public abstract Akka.Actor.IActorRef GetChild(System.Collections.Generic.IReadOnlyList<string> name);
public abstract void Restart(System.Exception cause);
public abstract void Resume(System.Exception causedByFailure = null);
[System.ObsoleteAttribute("Use SendSystemMessage(message) instead [1.1.0]")]
Expand Down Expand Up @@ -1245,7 +1243,7 @@ namespace Akka.Actor
protected Akka.Actor.IInternalActorRef Supervisor { get; }
protected Akka.Actor.ActorSystem System { get; }
public override Akka.Actor.ICell Underlying { get; }
public override Akka.Actor.IActorRef GetChild(System.Collections.Generic.IEnumerable<string> name) { }
public override Akka.Actor.IActorRef GetChild(System.Collections.Generic.IReadOnlyList<string> name) { }
public override Akka.Actor.IInternalActorRef GetSingleChild(string name) { }
protected virtual Akka.Actor.ActorCell NewActorCell(Akka.Actor.Internal.ActorSystemImpl system, Akka.Actor.IInternalActorRef self, Akka.Actor.Props props, Akka.Dispatch.MessageDispatcher dispatcher, Akka.Actor.IInternalActorRef supervisor) { }
public override void Restart(System.Exception cause) { }
Expand Down Expand Up @@ -1317,7 +1315,7 @@ namespace Akka.Actor
[System.ObsoleteAttribute("Use Context.Watch and Receive<Terminated> [1.1.0]")]
public override bool IsTerminated { get; }
public override Akka.Actor.IInternalActorRef Parent { get; }
public override Akka.Actor.IActorRef GetChild(System.Collections.Generic.IEnumerable<string> name) { }
public override Akka.Actor.IActorRef GetChild(System.Collections.Generic.IReadOnlyList<string> name) { }
public override void Restart(System.Exception cause) { }
public override void Resume(System.Exception causedByFailure = null) { }
public override void SendSystemMessage(Akka.Dispatch.SysMsg.ISystemMessage message) { }
Expand Down Expand Up @@ -1533,7 +1531,7 @@ namespace Akka.Actor
public override Akka.Actor.ActorPath Path { get; }
public override Akka.Actor.IActorRefProvider Provider { get; }
public override Akka.Actor.ICell Underlying { get; }
public override Akka.Actor.IActorRef GetChild(System.Collections.Generic.IEnumerable<string> name) { }
public override Akka.Actor.IActorRef GetChild(System.Collections.Generic.IReadOnlyList<string> name) { }
Copy link
Member Author

Choose a reason for hiding this comment

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

Source-compatible, but not necessarily a binary-compatible change.

public override Akka.Actor.IInternalActorRef GetSingleChild(string name) { }
public Akka.Actor.RepointableActorRef Initialize(bool async) { }
protected virtual Akka.Actor.ActorCell NewCell() { }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ namespace Akka.Remote
public override Akka.Actor.IInternalActorRef Parent { get; }
public override Akka.Actor.ActorPath Path { get; }
public override Akka.Actor.IActorRefProvider Provider { get; }
public override Akka.Actor.IActorRef GetChild(System.Collections.Generic.IEnumerable<string> name) { }
public override Akka.Actor.IActorRef GetChild(System.Collections.Generic.IReadOnlyList<string> name) { }
public bool IsWatchIntercepted(Akka.Actor.IActorRef watchee, Akka.Actor.IActorRef watcher) { }
public override void Restart(System.Exception cause) { }
public override void Resume(System.Exception causedByFailure = null) { }
Expand Down
10 changes: 5 additions & 5 deletions src/core/Akka.Remote/RemoteActorRef.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Akka.Annotations;
using Akka.Dispatch.SysMsg;
using Akka.Event;
using Akka.Util.Internal.Collections;

namespace Akka.Remote
{
Expand Down Expand Up @@ -106,17 +107,16 @@ public RemoteActorRef(RemoteTransport remote, Address localAddressToUse, ActorPa
/// <param name="name">The name.</param>
/// <returns>ActorRef.</returns>
/// <exception cref="System.NotImplementedException">TBD</exception>
public override IActorRef GetChild(IEnumerable<string> name)
public override IActorRef GetChild(IReadOnlyList<string> name)
{
var items = name.ToList();
switch (items.FirstOrDefault())
switch (name.FirstOrDefault())
{
case null:
return this;
case "..":
return Parent.GetChild(items.Skip(1));
return Parent.GetChild(name.NoCopySlice(1));
default:
return new RemoteActorRef(Remote, LocalAddressToUse, Path / items, ActorRefs.Nobody, Props.None, Deploy.None);
return new RemoteActorRef(Remote, LocalAddressToUse, Path / name, ActorRefs.Nobody, Props.None, Deploy.None);
}
}

Expand Down
10 changes: 4 additions & 6 deletions src/core/Akka.Remote/RemoteSystemDaemon.cs
Original file line number Diff line number Diff line change
Expand Up @@ -156,11 +156,9 @@ protected override void TellInternal(object message, IActorRef sender)
}
}

var t = Rec(ImmutableList<string>.Empty);
var concatenatedChildNames = t.Item1;
var m = t.Item2;
var (concatenatedChildNames, m) = Rec(ImmutableList<string>.Empty);

var child = GetChild(concatenatedChildNames);
var child = GetChild(concatenatedChildNames.ToList());
Copy link
Member Author

Choose a reason for hiding this comment

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

Extra allocation in remote deployments here.... might see if I can go about fixing that still....

if (child.IsNobody())
{
var emptyRef = new EmptyLocalActorRef(_system.Provider,
Expand Down Expand Up @@ -302,7 +300,7 @@ private void HandleDaemonMsgCreate(DaemonMsgCreate message)
/// </summary>
/// <param name="name">The name.</param>
/// <returns>ActorRef.</returns>
public override IActorRef GetChild(IEnumerable<string> name)
public override IActorRef GetChild(IReadOnlyList<string> name)
{
var path = name.Join("/");
var n = 0;
Expand All @@ -313,7 +311,7 @@ public override IActorRef GetChild(IEnumerable<string> name)
{
if (uid != ActorCell.UndefinedUid && uid != child.Path.Uid)
return Nobody.Instance;
return n == 0 ? child : child.GetChild(name.TakeRight(n));
return n == 0 ? child : child.GetChild(name.TakeRight(n).ToList());
Copy link
Member Author

Choose a reason for hiding this comment

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

Another allocation in remote deployments

}

var last = path.LastIndexOf("/", StringComparison.Ordinal);
Expand Down
2 changes: 1 addition & 1 deletion src/core/Akka.TestKit/TestActorRefBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ ISurrogate ISurrogated.ToSurrogate(ActorSystem system)

bool IInternalActorRef.IsTerminated { get { return _internalRef.IsTerminated; } }

IActorRef IInternalActorRef.GetChild(IEnumerable<string> name)
IActorRef IInternalActorRef.GetChild(IReadOnlyList<string> name)
{
return _internalRef.GetChild(name);
}
Expand Down
2 changes: 1 addition & 1 deletion src/core/Akka.TestKit/TestProbe.cs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ ISurrogate ISurrogated.ToSurrogate(ActorSystem system)

bool IInternalActorRef.IsTerminated { get { return ((IInternalActorRef)TestActor).IsTerminated; } }

IActorRef IInternalActorRef.GetChild(IEnumerable<string> name)
IActorRef IInternalActorRef.GetChild(IReadOnlyList<string> name)
{
return ((IInternalActorRef)TestActor).GetChild(name);
}
Expand Down
45 changes: 12 additions & 33 deletions src/core/Akka/Actor/ActorCell.Children.cs
Original file line number Diff line number Diff line change
Expand Up @@ -353,58 +353,37 @@ protected bool TryGetChildStatsByRef(IActorRef actor, out ChildRestartStats chil
/// </summary>
/// <param name="name">N/A</param>
/// <returns>N/A</returns>
[Obsolete("Use TryGetSingleChild [0.7.1]")]
public IInternalActorRef GetSingleChild(string name)
Copy link
Member Author

Choose a reason for hiding this comment

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

Half the perf optimizations happened here:

  • Simplified branching
  • Remove Try... out stuff since it was all functionally being converted back into ActorRefs.Nobody anyway.

{
IInternalActorRef child;
return TryGetSingleChild(name, out child) ? child : ActorRefs.Nobody;
}



/// <summary>
/// TBD
/// </summary>
/// <param name="name">TBD</param>
/// <param name="child">TBD</param>
/// <returns>TBD</returns>
public bool TryGetSingleChild(string name, out IInternalActorRef child)
{
if (name.IndexOf('#') < 0)
{
// optimization for the non-uid case
if (TryGetChildRestartStatsByName(name, out var stats))
if (ChildrenContainer.TryGetByName(name, out var stats) && stats is ChildRestartStats r)
{
child = stats.Child;
return true;
return r.Child;
}
else if (TryGetFunctionRef(name, out var functionRef))

if (TryGetFunctionRef(name, out var functionRef))
{
child = functionRef;
return true;
return functionRef;
}
}
else
{
var (s, uid) = GetNameAndUid(name);
if (TryGetChildRestartStatsByName(s, out var stats))
if (TryGetChildRestartStatsByName(s, out var stats) && (uid == ActorCell.UndefinedUid || uid == stats.Uid))
{
if (uid == ActorCell.UndefinedUid || uid == stats.Uid)
{
child = stats.Child;
return true;
}
return stats.Child;
}
else if (TryGetFunctionRef(s, uid, out var functionRef))

if (TryGetFunctionRef(s, uid, out var functionRef))
{
child = functionRef;
return true;
return functionRef;
}
}
child = ActorRefs.Nobody;
return false;
return ActorRefs.Nobody;
}

/// <summary>
/// TBD
/// </summary>
Expand Down
2 changes: 1 addition & 1 deletion src/core/Akka/Actor/ActorCell.cs
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ public void Init(bool sendSupervise, MailboxType mailboxType)
[Obsolete("Use TryGetChildStatsByName [0.7.1]", true)]
public IInternalActorRef GetChildByName(string name) //TODO: Should return Option[ChildStats]
{
return TryGetSingleChild(name, out var child) ? child : ActorRefs.Nobody;
return GetSingleChild(name);
}

IActorRef IActorContext.Child(string name)
Expand Down
22 changes: 10 additions & 12 deletions src/core/Akka/Actor/ActorRef.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
using Akka.Event;
using Akka.Util;
using Akka.Util.Internal;
using Akka.Util.Internal.Collections;

namespace Akka.Actor
{
Expand Down Expand Up @@ -422,7 +423,7 @@ public interface IInternalActorRef : IActorRef, IActorRefScope
/// </summary>
/// <param name="name">The path elements.</param>
/// <returns>The <see cref="IActorRef"/>, or if the requested path does not exist, returns <see cref="Nobody"/>.</returns>
IActorRef GetChild(IEnumerable<string> name);
IActorRef GetChild(IReadOnlyList<string> name);

/// <summary>
/// Resumes an actor if it has been suspended.
Expand Down Expand Up @@ -481,7 +482,7 @@ public abstract class InternalActorRefBase : ActorRefBase, IInternalActorRef
public abstract IActorRefProvider Provider { get; }

/// <inheritdoc cref="IInternalActorRef"/>
public abstract IActorRef GetChild(IEnumerable<string> name); //TODO: Refactor this to use an IEnumerator instead as this will be faster instead of enumerating multiple times over name, as the implementations currently do.
public abstract IActorRef GetChild(IReadOnlyList<string> name); //TODO: Refactor this to use an IEnumerator instead as this will be faster instead of enumerating multiple times over name, as the implementations currently do.

/// <inheritdoc cref="IInternalActorRef"/>
public abstract void Resume(Exception causedByFailure = null);
Expand Down Expand Up @@ -530,9 +531,9 @@ public override IInternalActorRef Parent
}

/// <inheritdoc cref="InternalActorRefBase"/>
public override IActorRef GetChild(IEnumerable<string> name)
public override IActorRef GetChild(IReadOnlyList<string> name)
{
if (name.All(string.IsNullOrEmpty))
if (name.All(x => string.IsNullOrEmpty(x)))
return this;
return ActorRefs.Nobody;
}
Expand Down Expand Up @@ -874,20 +875,17 @@ override def getChild(name: Iterator[String]): InternalActorRef = {
/// </summary>
/// <param name="name">TBD</param>
/// <returns>TBD</returns>
public override IActorRef GetChild(IEnumerable<string> name)
public override IActorRef GetChild(IReadOnlyList<string> name)
{
//Using enumerator to avoid multiple enumerations of name.
var enumerator = name.GetEnumerator();
if (!enumerator.MoveNext())
{
//name was empty
if (name.Count == 0)
return this;
}
var firstName = enumerator.Current;

var firstName = name[0];
if (string.IsNullOrEmpty(firstName))
return this;
if (_children.TryGetValue(firstName, out var child))
return child.GetChild(new Enumerable<string>(enumerator));
return child.GetChild(name.NoCopySlice(1));
return ActorRefs.Nobody;
}

Expand Down
2 changes: 1 addition & 1 deletion src/core/Akka/Actor/ActorRefProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ public IActorRef ResolveActorRef(ActorPath path)
/// <param name="actorRef">TBD</param>
/// <param name="pathElements">TBD</param>
/// <returns>TBD</returns>
internal IInternalActorRef ResolveActorRef(IInternalActorRef actorRef, IReadOnlyCollection<string> pathElements)
internal IInternalActorRef ResolveActorRef(IInternalActorRef actorRef, IReadOnlyList<string> pathElements)
{
if (pathElements.Count == 0)
{
Expand Down
Loading