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 2 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
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
4 changes: 2 additions & 2 deletions src/benchmark/Akka.Benchmarks/Actor/GetChildBenchmark.cs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ protected override void OnReceive(object message)
private ActorWithChild.Get _getMessage = new ActorWithChild.Get("food");
private ActorWithChild.Create _createMessage = new ActorWithChild.Create("food");

private ActorCell _cell;
private IActorContext _cell;
private RepointableActorRef _repointableActorRef;
private LocalActorRef _localActorRef;

Expand All @@ -118,7 +118,7 @@ public async Task Setup()
[Benchmark]
public void ResolveChild()
{
_cell.TryGetSingleChild(_getMessage.Name, out var child);
_cell.Child(_getMessage.Name);
}

[Benchmark]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1533,7 +1533,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(new ListSlice<string>(name, 1, name.Count-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
8 changes: 4 additions & 4 deletions src/core/Akka/Actor/ActorCell.Children.cs
Original file line number Diff line number Diff line change
Expand Up @@ -265,8 +265,7 @@ protected void SetTerminated()
{
get
{
var terminating = ChildrenContainer as TerminatingChildrenContainer;
return terminating != null && terminating.Reason is SuspendReason.IWaitingForChildren;
return ChildrenContainer is TerminatingChildrenContainer terminating && terminating.Reason is SuspendReason.IWaitingForChildren;
}
}

Expand Down Expand Up @@ -325,8 +324,7 @@ public bool TryGetChildStatsByName(string name, out IChildStats child) //This
/// </summary>
private bool TryGetChildRestartStatsByName(string name, out ChildRestartStats child)
{
IChildStats stats;
if (ChildrenContainer.TryGetByName(name, out stats))
if (ChildrenContainer.TryGetByName(name, out var stats))
{
child = stats as ChildRestartStats;
if (child != null)
Expand Down Expand Up @@ -362,6 +360,8 @@ public IInternalActorRef GetSingleChild(string name)
return TryGetSingleChild(name, out child) ? child : ActorRefs.Nobody;
}



/// <summary>
/// TBD
/// </summary>
Expand Down
5 changes: 4 additions & 1 deletion src/core/Akka/Actor/ActorCell.cs
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,10 @@ public void Init(bool sendSupervise, MailboxType mailboxType)

IActorRef IActorContext.Child(string name)
{
return TryGetSingleChild(name, out var child) ? child : ActorRefs.Nobody;
if (TryGetChildStatsByName(name, out var child) && child is ChildRestartStats s)
return s.Child;

return ActorRefs.Nobody;
}

/// <summary>
Expand Down
20 changes: 9 additions & 11 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,7 +531,7 @@ 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))
return this;
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(new ListSlice<string>(name, 1, name.Count-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
12 changes: 6 additions & 6 deletions src/core/Akka/Actor/LocalActorRef.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Akka.Dispatch;
using Akka.Dispatch.SysMsg;
using Akka.Util.Internal;
using Akka.Util.Internal.Collections;

namespace Akka.Actor
{
Expand Down Expand Up @@ -211,14 +212,13 @@ public override IInternalActorRef GetSingleChild(string name)
}

/// <inheritdoc/>
public override IActorRef GetChild(IEnumerable<string> name)
public override IActorRef GetChild(IReadOnlyList<string> name)
{
var current = (IActorRef)this;
IActorRef current = this;
int index = 0;
foreach (string element in name)
foreach (var element in name)
{
var currentLocalActorRef = current as LocalActorRef;
if (currentLocalActorRef != null)
if (current is LocalActorRef currentLocalActorRef)
{
switch (element)
{
Expand All @@ -237,7 +237,7 @@ public override IActorRef GetChild(IEnumerable<string> name)
//Current is not a LocalActorRef
if (current != null)
{
var rest = name.Skip(index).ToList();
var rest = new ListSlice<string>(name, 1, name.Count-1);
return current.AsInstanceOf<IInternalActorRef>().GetChild(rest);
}
throw new NotSupportedException("Bug, we should not get here");
Expand Down
15 changes: 8 additions & 7 deletions src/core/Akka/Actor/RepointableActorRef.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
using Akka.Dispatch.SysMsg;
using Akka.Event;
using Akka.Pattern;
using Akka.Util.Internal.Collections;

namespace Akka.Actor
{
Expand Down Expand Up @@ -278,17 +279,17 @@ protected override void TellInternal(object message, IActorRef sender)
/// </summary>
/// <param name="name">TBD</param>
/// <returns>TBD</returns>
public override IActorRef GetChild(IEnumerable<string> name)
public override IActorRef GetChild(IReadOnlyList<string> name)
{
var current = (IActorRef)this;
if (!name.Any()) return current;
IActorRef current = this;
if (name.Count == 0) return current;

var next = name.FirstOrDefault() ?? "";
var next = name[0];

switch (next)
{
case "..":
return Parent.GetChild(name.Skip(1));
return Parent.GetChild(new ListSlice<string>(name, 1, name.Count-1));
case "":
return ActorRefs.Nobody;
default:
Expand All @@ -297,8 +298,8 @@ public override IActorRef GetChild(IEnumerable<string> name)
{
if (stats is ChildRestartStats crs && (uid == ActorCell.UndefinedUid || uid == crs.Uid))
{
if (name.Skip(1).Any())
return crs.Child.GetChild(name.Skip(1));
if (name.Count > 1)
return crs.Child.GetChild(new ListSlice<string>(name, 1, name.Count-1));
else
return crs.Child;
}
Expand Down
Loading