From f25de15da83b4f1e09ad640e024414521f1d9220 Mon Sep 17 00:00:00 2001 From: zetanova Date: Tue, 7 Sep 2021 22:44:50 +0200 Subject: [PATCH 01/33] refactor remote-actorref-provider and add tests for cache entries --- src/core/Akka.Remote.Tests/RemotingSpec.cs | 29 +++++++++++++++++++ .../Akka.Remote/RemoteActorRefProvider.cs | 13 +++++---- .../Serialization/ActorPathCache.cs | 5 +++- .../Serialization/ActorRefResolveCache.cs | 5 +++- 4 files changed, 44 insertions(+), 8 deletions(-) diff --git a/src/core/Akka.Remote.Tests/RemotingSpec.cs b/src/core/Akka.Remote.Tests/RemotingSpec.cs index 4c3e354222b..7618698411e 100644 --- a/src/core/Akka.Remote.Tests/RemotingSpec.cs +++ b/src/core/Akka.Remote.Tests/RemotingSpec.cs @@ -23,6 +23,7 @@ using Xunit.Abstractions; using Nito.AsyncEx; using ThreadLocalRandom = Akka.Util.ThreadLocalRandom; +using Akka.Remote.Serialization; namespace Akka.Remote.Tests { @@ -177,6 +178,34 @@ public async Task Remoting_must_support_Ask() Assert.IsType>(actorRef); } + [Fact] + public async Task Remoting_should_not_cache_ref_of_local_ask() + { + var localActorRefResolveCache = ActorRefResolveThreadLocalCache.For(Sys); + var localActorPathCache = ActorPathThreadLocalCache.For(Sys); + + var (msg, actorRef) = await _here.Ask<(string, IActorRef)>("ping", DefaultTimeout); + Assert.Equal("pong", msg); + Assert.IsType>(actorRef); + + Assert.Equal(0, localActorRefResolveCache.All.Sum(n => n.Stats.Entries)); + Assert.Equal(2, localActorPathCache.All.Sum(n => n.Stats.Entries)); + } + + [Fact] + public async Task Remoting_should_not_cache_ref_of_remote_ask() + { + var remoteActorRefResolveCache = ActorRefResolveThreadLocalCache.For(_remoteSystem); + var remoteActorPathCache = ActorPathThreadLocalCache.For(_remoteSystem); + + var (msg, actorRef) = await _here.Ask<(string, IActorRef)>("ping", DefaultTimeout); + Assert.Equal("pong", msg); + Assert.IsType>(actorRef); + + Assert.Equal(0, remoteActorRefResolveCache.All.Sum(n => n.Stats.Entries)); + Assert.Equal(2, remoteActorPathCache.All.Sum(n => n.Stats.Entries)); //should be 1 + } + [Fact(Skip = "Racy")] public async Task Ask_does_not_deadlock() { diff --git a/src/core/Akka.Remote/RemoteActorRefProvider.cs b/src/core/Akka.Remote/RemoteActorRefProvider.cs index 0649dff960a..b2f6fe45c5e 100644 --- a/src/core/Akka.Remote/RemoteActorRefProvider.cs +++ b/src/core/Akka.Remote/RemoteActorRefProvider.cs @@ -238,8 +238,8 @@ public void UnregisterTempActor(ActorPath path) private volatile IActorRef _remotingTerminator; private volatile IActorRef _remoteWatcher; - private volatile ActorRefResolveThreadLocalCache _actorRefResolveThreadLocalCache; - private volatile ActorPathThreadLocalCache _actorPathThreadLocalCache; + private ActorRefResolveThreadLocalCache _actorRefResolveThreadLocalCache; + private ActorPathThreadLocalCache _actorPathThreadLocalCache; /// /// The remote death watcher. @@ -252,11 +252,11 @@ public virtual void Init(ActorSystemImpl system) { _system = system; - _local.Init(system); - _actorRefResolveThreadLocalCache = ActorRefResolveThreadLocalCache.For(system); _actorPathThreadLocalCache = ActorPathThreadLocalCache.For(system); + _local.Init(system); + _remotingTerminator = _system.SystemActorOf( RemoteSettings.ConfigureDispatcher(Props.Create(() => new RemotingTerminator(_local.SystemGuardian))), @@ -435,7 +435,7 @@ public Deploy LookUpRemotes(IEnumerable p) public bool HasAddress(Address address) { - return address == _local.RootPath.Address || address == RootPath.Address || Transport.Addresses.Any(a => a == address); + return address == _local.RootPath.Address || Transport.Addresses.Contains(address); } /// @@ -539,7 +539,8 @@ public IActorRef ResolveActorRef(string path) // if the value is not cached if (_actorRefResolveThreadLocalCache == null) { - return InternalResolveActorRef(path); // cache not initialized yet + // cache not initialized yet, should never happen + return InternalResolveActorRef(path); } return _actorRefResolveThreadLocalCache.Cache.GetOrCompute(path); } diff --git a/src/core/Akka.Remote/Serialization/ActorPathCache.cs b/src/core/Akka.Remote/Serialization/ActorPathCache.cs index f837bb0148d..1f292dcb1df 100644 --- a/src/core/Akka.Remote/Serialization/ActorPathCache.cs +++ b/src/core/Akka.Remote/Serialization/ActorPathCache.cs @@ -8,6 +8,7 @@ using System; using Akka.Actor; using System.Threading; +using System.Collections.Generic; namespace Akka.Remote.Serialization { @@ -16,10 +17,12 @@ namespace Akka.Remote.Serialization /// internal sealed class ActorPathThreadLocalCache : ExtensionIdProvider, IExtension { - private readonly ThreadLocal _current = new ThreadLocal(() => new ActorPathCache()); + private readonly ThreadLocal _current = new ThreadLocal(() => new ActorPathCache(), true); public ActorPathCache Cache => _current.Value; + internal IList All => _current.Values; + public override ActorPathThreadLocalCache CreateExtension(ExtendedActorSystem system) { return new ActorPathThreadLocalCache(); diff --git a/src/core/Akka.Remote/Serialization/ActorRefResolveCache.cs b/src/core/Akka.Remote/Serialization/ActorRefResolveCache.cs index 2bd7de5b453..4b650b6e894 100644 --- a/src/core/Akka.Remote/Serialization/ActorRefResolveCache.cs +++ b/src/core/Akka.Remote/Serialization/ActorRefResolveCache.cs @@ -5,6 +5,7 @@ // //----------------------------------------------------------------------- +using System.Collections.Generic; using System.Threading; using Akka.Actor; using Akka.Util.Internal; @@ -23,7 +24,7 @@ public ActorRefResolveThreadLocalCache() { } public ActorRefResolveThreadLocalCache(IRemoteActorRefProvider provider) { _provider = provider; - _current = new ThreadLocal(() => new ActorRefResolveCache(_provider)); + _current = new ThreadLocal(() => new ActorRefResolveCache(_provider), true); } public override ActorRefResolveThreadLocalCache CreateExtension(ExtendedActorSystem system) @@ -35,6 +36,8 @@ public override ActorRefResolveThreadLocalCache CreateExtension(ExtendedActorSys public ActorRefResolveCache Cache => _current.Value; + internal IList All => _current.Values; + public static ActorRefResolveThreadLocalCache For(ActorSystem system) { return system.WithExtension(); From 3fc01687508d4e3aab796e49c2a7776ca6428211 Mon Sep 17 00:00:00 2001 From: zetanova Date: Tue, 7 Sep 2021 23:40:46 +0200 Subject: [PATCH 02/33] replace address-cache with actorpath-cache --- src/core/Akka.Remote/Transport/AkkaPduCodec.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/Akka.Remote/Transport/AkkaPduCodec.cs b/src/core/Akka.Remote/Transport/AkkaPduCodec.cs index bde6e0baf27..98b39589f49 100644 --- a/src/core/Akka.Remote/Transport/AkkaPduCodec.cs +++ b/src/core/Akka.Remote/Transport/AkkaPduCodec.cs @@ -202,12 +202,12 @@ public AckAndMessage(Ack ackOption, Message messageOption) internal abstract class AkkaPduCodec { protected readonly ActorSystem System; - protected readonly AddressThreadLocalCache AddressCache; + protected readonly AddressThreadLocalCache ActorPathCache; protected AkkaPduCodec(ActorSystem system) { System = system; - AddressCache = AddressThreadLocalCache.For(system); + ActorPathCache = AddressThreadLocalCache.For(system); } /// @@ -427,9 +427,9 @@ public override AckAndMessage DecodeMessage(ByteString raw, IRemoteActorRefProvi { var recipient = provider.ResolveActorRefWithLocalAddress(envelopeContainer.Recipient.Path, localAddress); Address recipientAddress; - if (AddressCache != null) + if (ActorPathCache != null) { - recipientAddress = AddressCache.Cache.GetOrCompute(envelopeContainer.Recipient.Path); + recipientAddress = ActorPathCache.Cache.GetOrCompute(envelopeContainer.Recipient.Path); } else { From 9a67f1c542bcaf2e81e788fd04cd8e88554a7b72 Mon Sep 17 00:00:00 2001 From: zetanova Date: Tue, 7 Sep 2021 23:54:52 +0200 Subject: [PATCH 03/33] refactor resolve with local address --- .../Akka.Remote/RemoteActorRefProvider.cs | 51 +++++++++---------- .../Akka.Remote/Transport/AkkaPduCodec.cs | 4 +- 2 files changed, 26 insertions(+), 29 deletions(-) diff --git a/src/core/Akka.Remote/RemoteActorRefProvider.cs b/src/core/Akka.Remote/RemoteActorRefProvider.cs index b2f6fe45c5e..753d7dd11bc 100644 --- a/src/core/Akka.Remote/RemoteActorRefProvider.cs +++ b/src/core/Akka.Remote/RemoteActorRefProvider.cs @@ -433,9 +433,10 @@ public Deploy LookUpRemotes(IEnumerable p) return Deploy.None; } + [MethodImpl(MethodImplOptions.AggressiveInlining)] public bool HasAddress(Address address) { - return address == _local.RootPath.Address || Transport.Addresses.Contains(address); + return address == RootPath.Address || Transport.Addresses.Contains(address); } /// @@ -458,21 +459,6 @@ private IInternalActorRef LocalActorOf(ActorSystemImpl system, Props props, IInt return _local.ActorOf(system, props, supervisor, path, systemService, deploy, lookupDeploy, async); } - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private bool TryParseCachedPath(string actorPath, out ActorPath path) - { - if (_actorPathThreadLocalCache != null) - { - path = _actorPathThreadLocalCache.Cache.GetOrCompute(actorPath); - return path != null; - } - else // cache not initialized yet - { - return ActorPath.TryParse(actorPath, out path); - } - } - - /// /// INTERNAL API. /// @@ -483,20 +469,31 @@ private bool TryParseCachedPath(string actorPath, out ActorPath path) /// TBD public IInternalActorRef ResolveActorRefWithLocalAddress(string path, Address localAddress) { - if (TryParseCachedPath(path, out var actorPath)) + ActorPath actorPath; + if (_actorPathThreadLocalCache != null) { - //the actor's local address was already included in the ActorPath - if (HasAddress(actorPath.Address)) - { - if (actorPath is RootActorPath) - return RootGuardian; - return (IInternalActorRef)ResolveActorRef(path); // so we can use caching - } + actorPath = _actorPathThreadLocalCache.Cache.GetOrCompute(path); + } + else // cache not initialized yet + { + ActorPath.TryParse(path, out actorPath); + } - return CreateRemoteRef(new RootActorPath(actorPath.Address) / actorPath.ElementsWithUid, localAddress); + if (path is null) + { + _log.Debug("resolve of unknown path [{0}] failed", path); + return InternalDeadLetters; } - _log.Debug("resolve of unknown path [{0}] failed", path); - return InternalDeadLetters; + + if (!HasAddress(actorPath.Address)) + return CreateRemoteRef(new RootActorPath(actorPath.Address) / actorPath.ElementsWithUid, localAddress); + + //the actor's local address was already included in the ActorPath + + if (actorPath is RootActorPath) + return RootGuardian; + + return (IInternalActorRef)ResolveActorRef(path); // so we can use caching } diff --git a/src/core/Akka.Remote/Transport/AkkaPduCodec.cs b/src/core/Akka.Remote/Transport/AkkaPduCodec.cs index 98b39589f49..e0d9ffeae6d 100644 --- a/src/core/Akka.Remote/Transport/AkkaPduCodec.cs +++ b/src/core/Akka.Remote/Transport/AkkaPduCodec.cs @@ -431,9 +431,9 @@ public override AckAndMessage DecodeMessage(ByteString raw, IRemoteActorRefProvi { recipientAddress = ActorPathCache.Cache.GetOrCompute(envelopeContainer.Recipient.Path); } - else + else if (!ActorPath.TryParseAddress(envelopeContainer.Recipient.Path, out recipientAddress)) { - ActorPath.TryParseAddress(envelopeContainer.Recipient.Path, out recipientAddress); + recipientAddress = Address.AllSystems; } var serializedMessage = envelopeContainer.Message; From 746f76b0de30c6fb8d00a2ee53a1c1ece2d470a5 Mon Sep 17 00:00:00 2001 From: zetanova Date: Wed, 8 Sep 2021 00:13:17 +0200 Subject: [PATCH 04/33] refactor and cleanup --- .../Akka.Remote/RemoteActorRefProvider.cs | 25 ++++++++++--------- .../Akka.Remote/Transport/AkkaPduCodec.cs | 16 ++++-------- 2 files changed, 18 insertions(+), 23 deletions(-) diff --git a/src/core/Akka.Remote/RemoteActorRefProvider.cs b/src/core/Akka.Remote/RemoteActorRefProvider.cs index 753d7dd11bc..4a7423fbfdd 100644 --- a/src/core/Akka.Remote/RemoteActorRefProvider.cs +++ b/src/core/Akka.Remote/RemoteActorRefProvider.cs @@ -79,7 +79,7 @@ public interface IRemoteActorRefProvider : IActorRefProvider /// method. /// /// The path of the actor we intend to resolve. - /// An if a match was found. Otherwise nobody. + /// An if a match was found. Otherwise deadletters. IActorRef InternalResolveActorRef(string path); /// @@ -590,19 +590,20 @@ public IActorRef ResolveActorRef(ActorPath actorPath) /// The remote Address, if applicable. If not applicable null may be returned. public Address GetExternalAddressFor(Address address) { - if (HasAddress(address)) { return _local.RootPath.Address; } - if (!string.IsNullOrEmpty(address.Host) && address.Port.HasValue) + if (HasAddress(address)) + return _local.RootPath.Address; + + if (string.IsNullOrEmpty(address.Host) || !address.Port.HasValue) + return null; + + try { - try - { - return Transport.LocalAddressForRemote(address); - } - catch - { - return null; - } + return Transport.LocalAddressForRemote(address); + } + catch + { + return null; } - return null; } /// diff --git a/src/core/Akka.Remote/Transport/AkkaPduCodec.cs b/src/core/Akka.Remote/Transport/AkkaPduCodec.cs index e0d9ffeae6d..8e257d47a55 100644 --- a/src/core/Akka.Remote/Transport/AkkaPduCodec.cs +++ b/src/core/Akka.Remote/Transport/AkkaPduCodec.cs @@ -426,22 +426,15 @@ public override AckAndMessage DecodeMessage(ByteString raw, IRemoteActorRefProvi if (envelopeContainer != null) { var recipient = provider.ResolveActorRefWithLocalAddress(envelopeContainer.Recipient.Path, localAddress); - Address recipientAddress; - if (ActorPathCache != null) - { - recipientAddress = ActorPathCache.Cache.GetOrCompute(envelopeContainer.Recipient.Path); - } - else if (!ActorPath.TryParseAddress(envelopeContainer.Recipient.Path, out recipientAddress)) - { - recipientAddress = Address.AllSystems; - } + + //todo get parsed address from provider + var recipientAddress = ActorPathCache.Cache.GetOrCompute(envelopeContainer.Recipient.Path); var serializedMessage = envelopeContainer.Message; IActorRef senderOption = null; if (envelopeContainer.Sender != null) - { senderOption = provider.ResolveActorRefWithLocalAddress(envelopeContainer.Sender.Path, localAddress); - } + SeqNo seqOption = null; if (envelopeContainer.Seq != SeqUndefined) { @@ -450,6 +443,7 @@ public override AckAndMessage DecodeMessage(ByteString raw, IRemoteActorRefProvi seqOption = new SeqNo((long)envelopeContainer.Seq); //proto takes a ulong } } + messageOption = new Message(recipient, recipientAddress, serializedMessage, senderOption, seqOption); } } From bff01bb2895a824be1127c46e6a06363c81e21f6 Mon Sep 17 00:00:00 2001 From: zetanova Date: Wed, 8 Sep 2021 00:45:52 +0200 Subject: [PATCH 05/33] remove volatile from fields --- src/core/Akka.Remote/RemoteActorRefProvider.cs | 10 +++++----- src/core/Akka.Remote/RemoteSystemDaemon.cs | 4 ++-- src/core/Akka.Remote/Remoting.cs | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/core/Akka.Remote/RemoteActorRefProvider.cs b/src/core/Akka.Remote/RemoteActorRefProvider.cs index 4a7423fbfdd..2c71c221250 100644 --- a/src/core/Akka.Remote/RemoteActorRefProvider.cs +++ b/src/core/Akka.Remote/RemoteActorRefProvider.cs @@ -128,7 +128,7 @@ public RemoteActorRefProvider(string systemName, Settings settings, EventStream } private readonly LocalActorRefProvider _local; - private volatile Internals _internals; + private Internals _internals; private ActorSystemImpl _system; private Internals RemoteInternals @@ -235,8 +235,8 @@ public void UnregisterTempActor(ActorPath path) _local.UnregisterTempActor(path); } - private volatile IActorRef _remotingTerminator; - private volatile IActorRef _remoteWatcher; + private IActorRef _remotingTerminator; + private IActorRef _remoteWatcher; private ActorRefResolveThreadLocalCache _actorRefResolveThreadLocalCache; private ActorPathThreadLocalCache _actorPathThreadLocalCache; @@ -245,7 +245,7 @@ public void UnregisterTempActor(ActorPath path) /// The remote death watcher. /// public IActorRef RemoteWatcher => _remoteWatcher; - private volatile IActorRef _remoteDeploymentWatcher; + private IActorRef _remoteDeploymentWatcher; /// public virtual void Init(ActorSystemImpl system) @@ -262,7 +262,7 @@ public virtual void Init(ActorSystemImpl system) RemoteSettings.ConfigureDispatcher(Props.Create(() => new RemotingTerminator(_local.SystemGuardian))), "remoting-terminator"); - _internals = CreateInternals(); + _internals = CreateInternals(); _remotingTerminator.Tell(RemoteInternals); diff --git a/src/core/Akka.Remote/RemoteSystemDaemon.cs b/src/core/Akka.Remote/RemoteSystemDaemon.cs index 2e5fce15820..d1dc349ccab 100644 --- a/src/core/Akka.Remote/RemoteSystemDaemon.cs +++ b/src/core/Akka.Remote/RemoteSystemDaemon.cs @@ -28,7 +28,7 @@ internal interface IDaemonMsg { } /// /// INTERNAL API /// - internal class DaemonMsgCreate : IDaemonMsg + internal sealed class DaemonMsgCreate : IDaemonMsg { /// /// Initializes a new instance of the class. @@ -77,7 +77,7 @@ public DaemonMsgCreate(Props props, Deploy deploy, string path, IActorRef superv /// /// It acts as the brain of the remote that responds to system remote messages and executes actions accordingly. /// - internal class RemoteSystemDaemon : VirtualPathContainer + internal sealed class RemoteSystemDaemon : VirtualPathContainer { private readonly ActorSystemImpl _system; private readonly Switch _terminating = new Switch(false); diff --git a/src/core/Akka.Remote/Remoting.cs b/src/core/Akka.Remote/Remoting.cs index 25e4eb6cdad..28242173e71 100644 --- a/src/core/Akka.Remote/Remoting.cs +++ b/src/core/Akka.Remote/Remoting.cs @@ -111,7 +111,7 @@ internal interface IPriorityMessage { } /// /// INTERNAL API /// - internal class Remoting : RemoteTransport + internal sealed class Remoting : RemoteTransport { private readonly ILoggingAdapter _log; private volatile IDictionary> _transportMapping; From 2a55502d349594b1817157f58b558421f66313af Mon Sep 17 00:00:00 2001 From: zetanova Date: Wed, 8 Sep 2021 17:31:02 +0200 Subject: [PATCH 06/33] remove double equals --- src/core/Akka.Remote/RemoteActorRefProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/Akka.Remote/RemoteActorRefProvider.cs b/src/core/Akka.Remote/RemoteActorRefProvider.cs index b063d601adf..325adacdbda 100644 --- a/src/core/Akka.Remote/RemoteActorRefProvider.cs +++ b/src/core/Akka.Remote/RemoteActorRefProvider.cs @@ -436,7 +436,7 @@ public Deploy LookUpRemotes(IEnumerable p) [MethodImpl(MethodImplOptions.AggressiveInlining)] public bool HasAddress(Address address) { - return address.Equals(_local.RootPath.Address) || address.Equals(RootPath.Address) || Transport.Addresses.Contains(address); + return address.Equals(RootPath.Address) || Transport.Addresses.Contains(address); } /// From 14e88e26669914b28a2a638aaa19e688a855608a Mon Sep 17 00:00:00 2001 From: zetanova Date: Fri, 10 Sep 2021 00:00:48 +0200 Subject: [PATCH 07/33] cleanup --- src/core/Akka/Actor/ActorPath.cs | 129 +++++++++++++------------------ 1 file changed, 52 insertions(+), 77 deletions(-) diff --git a/src/core/Akka/Actor/ActorPath.cs b/src/core/Akka/Actor/ActorPath.cs index 8491dd1e886..bbf5e6afe95 100644 --- a/src/core/Akka/Actor/ActorPath.cs +++ b/src/core/Akka/Actor/ActorPath.cs @@ -11,7 +11,6 @@ using System.Linq; using Akka.Util; using Newtonsoft.Json; -using static System.String; namespace Akka.Actor { @@ -36,7 +35,7 @@ public abstract class ActorPath : IEquatable, IComparable, /// This class represents a surrogate of an . /// Its main use is to help during the serialization process. /// - public class Surrogate : ISurrogate, IEquatable, IEquatable + public sealed class Surrogate : ISurrogate, IEquatable, IEquatable { /// /// Initializes a new instance of the class. @@ -59,12 +58,7 @@ public Surrogate(string path) /// The encapsulated by this surrogate. public ISurrogated FromSurrogate(ActorSystem system) { - if (TryParse(Path, out var path)) - { - return path; - } - - return null; + return TryParse(Path, out var path) ? path : null; } #region Equality @@ -72,25 +66,23 @@ public ISurrogated FromSurrogate(ActorSystem system) /// public bool Equals(Surrogate other) { - if (ReferenceEquals(null, other)) return false; - if (ReferenceEquals(this, other)) return true; - return string.Equals(Path, other.Path); + if (other is null) return false; + return ReferenceEquals(this, other) || StringComparer.Ordinal.Equals(Path, other.Path); } /// public bool Equals(ActorPath other) { - if (other == null) return false; + if (other is null) return false; return Equals(other.ToSurrogate(null)); //TODO: not so sure if this is OK } /// public override bool Equals(object obj) { - if (ReferenceEquals(null, obj)) return false; + if (obj is null) return false; if (ReferenceEquals(this, obj)) return true; - var actorPath = obj as ActorPath; - if (actorPath != null) return Equals(actorPath); + if (obj is ActorPath actorPath) return Equals(actorPath); return Equals(obj as Surrogate); } @@ -117,11 +109,7 @@ public override int GetHashCode() /// TBD public static bool IsValidPathElement(string s) { - if (IsNullOrEmpty(s)) - { - return false; - } - return !s.StartsWith("$") && Validate(s); + return !string.IsNullOrEmpty(s) && !s.StartsWith("$") && Validate(s); } private static bool IsValidChar(char c) => (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || @@ -138,11 +126,11 @@ private static bool Validate(string chars) { if (IsValidChar(chars[pos])) { - pos = pos + 1; + pos += 1; } else if (chars[pos] == '%' && pos + 2 < len && IsHexChar(chars[pos + 1]) && IsHexChar(chars[pos + 2])) { - pos = pos + 3; + pos += 3; } else { @@ -159,21 +147,21 @@ private static bool Validate(string chars) /// The name. protected ActorPath(Address address, string name) { - Name = name; Address = address; + Name = name; } /// /// Initializes a new instance of the class. /// - /// The parent path. + /// The address. /// The name. /// The uid. - protected ActorPath(ActorPath parentPath, string name, long uid) + protected ActorPath(Address address, string name, long uid) { - Address = parentPath.Address; - Uid = uid; + Address = address; Name = name; + Uid = uid; } /// @@ -182,8 +170,6 @@ protected ActorPath(ActorPath parentPath, string name, long uid) /// The uid. public long Uid { get; } - internal static readonly string[] EmptyElements = { }; - /// /// Gets the elements. /// @@ -202,7 +188,7 @@ internal IReadOnlyList ElementsWithUid { get { - if (this is RootActorPath) return EmptyElements; + if (this is RootActorPath) return Array.Empty(); var elements = (List)Elements; elements[elements.Count - 1] = AppendUidFragment(Name); return elements; @@ -243,7 +229,7 @@ public bool Equals(ActorPath other) ActorPath a = this; ActorPath b = other; - for (; ; ) + while (true) { if (ReferenceEquals(a, b)) return true; @@ -308,12 +294,10 @@ public bool Equals(ActorPath other) /// A newly created public static ActorPath Parse(string path) { - ActorPath actorPath; - if (TryParse(path, out actorPath)) - { - return actorPath; - } - throw new UriFormatException($"Can not parse an ActorPath: {path}"); + if (!TryParse(path, out var actorPath)) + throw new UriFormatException($"Can not parse an ActorPath: {path}"); + + return actorPath; } /// @@ -325,13 +309,14 @@ public static ActorPath Parse(string path) /// TBD public static bool TryParse(string path, out ActorPath actorPath) { - actorPath = null; - - if (!TryParseAddress(path, out var address, out var absoluteUri)) return false; - var spanified = absoluteUri; + if (!TryParseAddress(path, out var address, out var spanified)) + { + actorPath = null; + return false; + } // check for Uri fragment here - var nextSlash = 0; + int nextSlash; actorPath = new RootActorPath(address); @@ -347,7 +332,7 @@ public static bool TryParse(string path, out ActorPath actorPath) var fragLoc = spanified.IndexOf('#'); if (fragLoc > -1) { - var fragment = spanified.Slice(fragLoc+1); + var fragment = spanified.Slice(fragLoc + 1); var fragValue = SpanHacks.Parse(fragment); spanified = spanified.Slice(0, fragLoc); actorPath = new ChildActorPath(actorPath, spanified.ToString(), fragValue); @@ -356,7 +341,7 @@ public static bool TryParse(string path, out ActorPath actorPath) { actorPath /= spanified.ToString(); } - + } spanified = spanified.Slice(nextSlash + 1); @@ -405,7 +390,7 @@ private static bool TryParseAddress(string path, out Address address, out ReadOn spanified = spanified.Slice(2); // move past the double // var firstAtPos = spanified.IndexOf('@'); - var sysName = string.Empty; + string sysName; if (firstAtPos == -1) { // dealing with an absolute local Uri @@ -435,7 +420,7 @@ private static bool TryParseAddress(string path, out Address address, out ReadOn * - IPV4 / hostnames * - IPV6 (must be surrounded by '[]') according to spec. */ - var host = string.Empty; + string host; // check for IPV6 first var openBracket = spanified.IndexOf('['); @@ -555,10 +540,7 @@ public override string ToString() /// TBD public string ToStringWithUid() { - var uid = Uid; - if (uid == ActorCell.UndefinedUid) - return ToStringWithAddress(); - return ToStringWithAddress() + "#" + uid; + return Uid != ActorCell.UndefinedUid ? $"{ToStringWithAddress()}#{Uid}" : ToStringWithAddress(); } /// @@ -587,8 +569,7 @@ public override int GetHashCode() /// public override bool Equals(object obj) { - var other = obj as ActorPath; - return Equals(other); + return Equals(obj as ActorPath); } /// @@ -650,10 +631,7 @@ public string ToSerializationFormatWithAddress(Address address) private string AppendUidFragment(string withAddress) { - if (Uid == ActorCell.UndefinedUid) - return withAddress; - - return String.Concat(withAddress, "#", Uid.ToString()); + return Uid != ActorCell.UndefinedUid ? $"{withAddress}#{Uid}" : withAddress; } /// @@ -683,7 +661,7 @@ public string ToStringWithAddress(Address address) /// TBD public static string FormatPathElements(IEnumerable pathElements) { - return String.Join("/", pathElements); + return string.Join("/", pathElements); } /// @@ -700,7 +678,7 @@ public ISurrogate ToSurrogate(ActorSystem system) /// /// Actor paths for root guardians, such as "/user" and "/system" /// - public class RootActorPath : ActorPath + public sealed class RootActorPath : ActorPath { /// /// Initializes a new instance of the class. @@ -715,7 +693,7 @@ public RootActorPath(Address address, string name = "") /// public override ActorPath Parent => null; - public override IReadOnlyList Elements => EmptyElements; + public override IReadOnlyList Elements => Array.Empty(); /// [JsonIgnore] @@ -724,25 +702,25 @@ public RootActorPath(Address address, string name = "") /// public override ActorPath WithUid(long uid) { - if (uid == 0) - return this; - throw new NotSupportedException("RootActorPath must have undefined Uid"); + if (uid != 0) + throw new NotSupportedException("RootActorPath must have undefined Uid"); + + return this; } /// public override int CompareTo(ActorPath other) { if (other is ChildActorPath) return 1; - return Compare(ToString(), other.ToString(), StringComparison.Ordinal); + return StringComparer.Ordinal.Compare(ToString(), other?.ToString()); } } /// /// Actor paths for child actors, which is to say any non-guardian actor. /// - public class ChildActorPath : ActorPath + public sealed class ChildActorPath : ActorPath { - private readonly string _name; private readonly ActorPath _parent; /// @@ -752,9 +730,8 @@ public class ChildActorPath : ActorPath /// The name. /// The uid. public ChildActorPath(ActorPath parentPath, string name, long uid) - : base(parentPath, name, uid) + : base(parentPath.Address, name, uid) { - _name = name; _parent = parentPath; } @@ -784,9 +761,7 @@ public override ActorPath Root { var current = _parent; while (current is ChildActorPath child) - { current = child._parent; - } return current.Root; } } @@ -798,9 +773,7 @@ public override ActorPath Root /// ActorPath. public override ActorPath WithUid(long uid) { - if (uid == Uid) - return this; - return new ChildActorPath(_parent, _name, uid); + return uid != Uid ? new ChildActorPath(_parent, Name, uid) : this; } /// @@ -825,15 +798,17 @@ public override int CompareTo(ActorPath other) private int InternalCompareTo(ActorPath left, ActorPath right) { if (ReferenceEquals(left, right)) return 0; - var leftRoot = left as RootActorPath; - if (leftRoot != null) + + if (left is RootActorPath leftRoot) return leftRoot.CompareTo(right); - var rightRoot = right as RootActorPath; - if (rightRoot != null) + + if (right is RootActorPath rightRoot) return -rightRoot.CompareTo(left); - var nameCompareResult = Compare(left.Name, right.Name, StringComparison.Ordinal); + + var nameCompareResult = StringComparer.Ordinal.Compare(left.Name, right.Name); if (nameCompareResult != 0) return nameCompareResult; + return InternalCompareTo(left.Parent, right.Parent); } } From 762ec30513292a1ffef0a797cc39d1ce28163d10 Mon Sep 17 00:00:00 2001 From: zetanova Date: Fri, 10 Sep 2021 00:57:20 +0200 Subject: [PATCH 08/33] refactor to base --- src/core/Akka/Actor/ActorPath.cs | 236 +++++++++++++------------------ 1 file changed, 102 insertions(+), 134 deletions(-) diff --git a/src/core/Akka/Actor/ActorPath.cs b/src/core/Akka/Actor/ActorPath.cs index bbf5e6afe95..33246200450 100644 --- a/src/core/Akka/Actor/ActorPath.cs +++ b/src/core/Akka/Actor/ActorPath.cs @@ -140,6 +140,11 @@ private static bool Validate(string chars) return true; } + private readonly Address _address; + private readonly ActorPath _parent; + private readonly string _name; + private readonly long _uid; + /// /// Initializes a new instance of the class. /// @@ -147,34 +152,67 @@ private static bool Validate(string chars) /// The name. protected ActorPath(Address address, string name) { - Address = address; - Name = name; + _address = address; + _name = name; } /// /// Initializes a new instance of the class. /// - /// The address. + /// The parentPath. /// The name. /// The uid. - protected ActorPath(Address address, string name, long uid) + protected ActorPath(ActorPath parentPath, string name, long uid) { - Address = address; - Name = name; - Uid = uid; + _parent = parentPath; + _address = parentPath.Address; + _name = name; + _uid = uid; } + /// + /// Gets the name. + /// + /// The name. + public string Name => _name; + + /// + /// The Address under which this path can be reached; walks up the tree to + /// the RootActorPath. + /// + /// The address. + public Address Address => _address; + /// /// Gets the uid. /// /// The uid. - public long Uid { get; } + public long Uid => _uid; + + /// + /// The path of the parent to this actor. + /// + public ActorPath Parent => _parent; /// /// Gets the elements. /// /// The elements. - public abstract IReadOnlyList Elements { get; } + public IReadOnlyList Elements + { + get + { + var acc = new Stack(); + var p = this; + while (true) + { + if (p.IsRoot) + return acc.ToList(); + acc.Push(p.Name); + p = p.Parent; + } + } + } /// /// INTERNAL API. @@ -195,28 +233,28 @@ internal IReadOnlyList ElementsWithUid } } - /// - /// Gets the name. - /// - /// The name. - public string Name { get; } - /// - /// The Address under which this path can be reached; walks up the tree to - /// the RootActorPath. - /// - /// The address. - public Address Address { get; } /// /// The root actor path. /// - public abstract ActorPath Root { get; } + [JsonIgnore] + public ActorPath Root + { + get + { + var current = this; + while (current._parent is ActorPath p) + current = p; + return current; + } + } /// - /// The path of the parent to this actor. + /// Is this instance the root actor path. /// - public abstract ActorPath Parent { get; } + [JsonIgnore] + public bool IsRoot => _parent is null; /// public bool Equals(ActorPath other) @@ -244,14 +282,49 @@ public bool Equals(ActorPath other) } /// - public abstract int CompareTo(ActorPath other); + public int CompareTo(ActorPath other) + { + if (IsRoot) + { + if (other is ChildActorPath) return 1; + return StringComparer.Ordinal.Compare(ToString(), other?.ToString()); + } + return InternalCompareTo(this, other); + } + + private int InternalCompareTo(ActorPath left, ActorPath right) + { + if (ReferenceEquals(left, right)) + return 0; + + if (left.IsRoot) + return left.CompareTo(right); + + if (right.IsRoot) + return -right.CompareTo(left); + + var nameCompareResult = StringComparer.Ordinal.Compare(left.Name, right.Name); + if (nameCompareResult != 0) + return nameCompareResult; + + return InternalCompareTo(left.Parent, right.Parent); + } /// - /// Withes the uid. + /// Creates a copy of the given ActorPath and applies a new Uid /// /// The uid. /// ActorPath. - public abstract ActorPath WithUid(long uid); + public ActorPath WithUid(long uid) + { + if (IsRoot) + { + if (uid != 0) throw new NotSupportedException("RootActorPath must have undefined Uid"); + return this; + } + + return uid != Uid ? new ChildActorPath(_parent, Name, uid) : this; + } /// /// Creates a new with the specified parent @@ -553,15 +626,14 @@ public ActorPath Child(string childName) return this / childName; } - /// public override int GetHashCode() { unchecked { var hash = 17; hash = (hash * 23) ^ Address.GetHashCode(); - foreach (var e in Elements) - hash = (hash * 23) ^ e.GetHashCode(); + for (var p = this; !(p is null); p = p.Parent) + hash = (hash * 23) ^ p.Name.GetHashCode(); return hash; } } @@ -690,30 +762,6 @@ public RootActorPath(Address address, string name = "") { } - /// - public override ActorPath Parent => null; - - public override IReadOnlyList Elements => Array.Empty(); - - /// - [JsonIgnore] - public override ActorPath Root => this; - - /// - public override ActorPath WithUid(long uid) - { - if (uid != 0) - throw new NotSupportedException("RootActorPath must have undefined Uid"); - - return this; - } - - /// - public override int CompareTo(ActorPath other) - { - if (other is ChildActorPath) return 1; - return StringComparer.Ordinal.Compare(ToString(), other?.ToString()); - } } /// @@ -721,8 +769,6 @@ public override int CompareTo(ActorPath other) /// public sealed class ChildActorPath : ActorPath { - private readonly ActorPath _parent; - /// /// Initializes a new instance of the class. /// @@ -730,86 +776,8 @@ public sealed class ChildActorPath : ActorPath /// The name. /// The uid. public ChildActorPath(ActorPath parentPath, string name, long uid) - : base(parentPath.Address, name, uid) - { - _parent = parentPath; - } - - /// - public override ActorPath Parent => _parent; - - public override IReadOnlyList Elements + : base(parentPath, name, uid) { - get - { - ActorPath p = this; - var acc = new Stack(); - while (true) - { - if (p is RootActorPath) - return acc.ToList(); - acc.Push(p.Name); - p = p.Parent; - } - } - } - - /// - public override ActorPath Root - { - get - { - var current = _parent; - while (current is ChildActorPath child) - current = child._parent; - return current.Root; - } - } - - /// - /// Creates a copy of the given ActorPath and applies a new Uid - /// - /// The uid. - /// ActorPath. - public override ActorPath WithUid(long uid) - { - return uid != Uid ? new ChildActorPath(_parent, Name, uid) : this; - } - - /// - public override int GetHashCode() - { - unchecked - { - var hash = 17; - hash = (hash * 23) ^ Address.GetHashCode(); - for (ActorPath p = this; p != null; p = p.Parent) - hash = (hash * 23) ^ p.Name.GetHashCode(); - return hash; - } - } - - /// - public override int CompareTo(ActorPath other) - { - return InternalCompareTo(this, other); - } - - private int InternalCompareTo(ActorPath left, ActorPath right) - { - if (ReferenceEquals(left, right)) return 0; - - if (left is RootActorPath leftRoot) - return leftRoot.CompareTo(right); - - if (right is RootActorPath rightRoot) - return -rightRoot.CompareTo(left); - - var nameCompareResult = StringComparer.Ordinal.Compare(left.Name, right.Name); - if (nameCompareResult != 0) - return nameCompareResult; - - return InternalCompareTo(left.Parent, right.Parent); } } } From aea216692a91126108d589e181c51bab2fff477c Mon Sep 17 00:00:00 2001 From: zetanova Date: Fri, 10 Sep 2021 01:45:21 +0200 Subject: [PATCH 09/33] optimize elements list --- src/core/Akka/Actor/ActorPath.cs | 70 +++++++++++++++++++------------- 1 file changed, 42 insertions(+), 28 deletions(-) diff --git a/src/core/Akka/Actor/ActorPath.cs b/src/core/Akka/Actor/ActorPath.cs index 33246200450..21329824909 100644 --- a/src/core/Akka/Actor/ActorPath.cs +++ b/src/core/Akka/Actor/ActorPath.cs @@ -142,22 +142,27 @@ private static bool Validate(string chars) private readonly Address _address; private readonly ActorPath _parent; + private readonly int _depth; + private readonly string _name; private readonly long _uid; /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class as root. /// /// The address. /// The name. protected ActorPath(Address address, string name) { _address = address; + _parent = null; + _depth = 0; _name = name; + _uid = ActorCell.UndefinedUid; } /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class as child. /// /// The parentPath. /// The name. @@ -166,6 +171,7 @@ protected ActorPath(ActorPath parentPath, string name, long uid) { _parent = parentPath; _address = parentPath.Address; + _depth = parentPath._depth + 1; _name = name; _uid = uid; } @@ -194,6 +200,11 @@ protected ActorPath(ActorPath parentPath, string name, long uid) /// public ActorPath Parent => _parent; + /// + /// The the depth of the actor. + /// + public int Depth => _depth; + /// /// Gets the elements. /// @@ -202,15 +213,18 @@ public IReadOnlyList Elements { get { - var acc = new Stack(); + if (_depth == 0) + return ImmutableArray.Empty; + + var b = ImmutableArray.CreateBuilder(_depth); + b.Count = _depth; var p = this; - while (true) + for(var i = 0; i < _depth; i++) { - if (p.IsRoot) - return acc.ToList(); - acc.Push(p.Name); - p = p.Parent; + b[_depth - i - 1] = p.Name; + p = p._parent; } + return b.ToImmutable(); } } @@ -226,15 +240,21 @@ internal IReadOnlyList ElementsWithUid { get { - if (this is RootActorPath) return Array.Empty(); - var elements = (List)Elements; - elements[elements.Count - 1] = AppendUidFragment(Name); - return elements; + if (_depth == 0) + return ImmutableArray.Empty; + + var b = ImmutableArray.CreateBuilder(_depth); + b.Count = _depth; + var p = this; + for (var i = 0; i < _depth; i++) + { + b[_depth - i - 1] = i > 0 ? p.Name : AppendUidFragment(p.Name); + p = p._parent; + } + return b.ToImmutable(); } } - - /// /// The root actor path. /// @@ -244,18 +264,12 @@ public ActorPath Root get { var current = this; - while (current._parent is ActorPath p) - current = p; + while (current._depth > 0) + current = current.Parent; return current; } } - /// - /// Is this instance the root actor path. - /// - [JsonIgnore] - public bool IsRoot => _parent is null; - /// public bool Equals(ActorPath other) { @@ -284,7 +298,7 @@ public bool Equals(ActorPath other) /// public int CompareTo(ActorPath other) { - if (IsRoot) + if (_depth == 0) { if (other is ChildActorPath) return 1; return StringComparer.Ordinal.Compare(ToString(), other?.ToString()); @@ -297,17 +311,17 @@ private int InternalCompareTo(ActorPath left, ActorPath right) if (ReferenceEquals(left, right)) return 0; - if (left.IsRoot) + if (left._depth == 0) return left.CompareTo(right); - if (right.IsRoot) + if (right._depth == 0) return -right.CompareTo(left); - var nameCompareResult = StringComparer.Ordinal.Compare(left.Name, right.Name); + var nameCompareResult = StringComparer.Ordinal.Compare(left._name, right._name); if (nameCompareResult != 0) return nameCompareResult; - return InternalCompareTo(left.Parent, right.Parent); + return InternalCompareTo(left._parent, right._parent); } /// @@ -317,7 +331,7 @@ private int InternalCompareTo(ActorPath left, ActorPath right) /// ActorPath. public ActorPath WithUid(long uid) { - if (IsRoot) + if (_depth == 0) { if (uid != 0) throw new NotSupportedException("RootActorPath must have undefined Uid"); return this; From a36170f63eb81dbcf1e48b3cb9fdf6e9fe4498b8 Mon Sep 17 00:00:00 2001 From: zetanova Date: Fri, 10 Sep 2021 03:00:34 +0200 Subject: [PATCH 10/33] improve actor path join --- src/core/Akka.Tests/Actor/ActorPathSpec.cs | 12 +-- src/core/Akka/Actor/ActorPath.cs | 92 +++++++++++++--------- src/core/Akka/Actor/Address.cs | 2 +- 3 files changed, 60 insertions(+), 46 deletions(-) diff --git a/src/core/Akka.Tests/Actor/ActorPathSpec.cs b/src/core/Akka.Tests/Actor/ActorPathSpec.cs index acd081c6494..31d51313842 100644 --- a/src/core/Akka.Tests/Actor/ActorPathSpec.cs +++ b/src/core/Akka.Tests/Actor/ActorPathSpec.cs @@ -103,12 +103,12 @@ public void Supports_parsing_remote_FQDN_paths() [Fact] public void Return_false_upon_malformed_path() { - ActorPath ignored; - ActorPath.TryParse("", out ignored).ShouldBe(false); - ActorPath.TryParse("://hallo", out ignored).ShouldBe(false); - ActorPath.TryParse("s://dd@:12", out ignored).ShouldBe(false); - ActorPath.TryParse("s://dd@h:hd", out ignored).ShouldBe(false); - ActorPath.TryParse("a://l:1/b", out ignored).ShouldBe(false); + ActorPath.TryParse("", out _).ShouldBe(false); + ActorPath.TryParse("://hallo", out _).ShouldBe(false); + ActorPath.TryParse("s://dd@:12", out _).ShouldBe(false); + ActorPath.TryParse("s://dd@h:hd", out _).ShouldBe(false); + ActorPath.TryParse("a://l:1/b", out _).ShouldBe(false); + ActorPath.TryParse("akka:/", out _).ShouldBe(false); } [Fact] diff --git a/src/core/Akka/Actor/ActorPath.cs b/src/core/Akka/Actor/ActorPath.cs index 21329824909..6e4bff6df77 100644 --- a/src/core/Akka/Actor/ActorPath.cs +++ b/src/core/Akka/Actor/ActorPath.cs @@ -219,7 +219,7 @@ public IReadOnlyList Elements var b = ImmutableArray.CreateBuilder(_depth); b.Count = _depth; var p = this; - for(var i = 0; i < _depth; i++) + for (var i = 0; i < _depth; i++) { b[_depth - i - 1] = p.Name; p = p._parent; @@ -248,7 +248,7 @@ internal IReadOnlyList ElementsWithUid var p = this; for (var i = 0; i < _depth; i++) { - b[_depth - i - 1] = i > 0 ? p.Name : AppendUidFragment(p.Name); + b[_depth - i - 1] = i > 0 ? p._name : AppendUidFragment(p._name); p = p._parent; } return b.ToImmutable(); @@ -396,6 +396,8 @@ public static ActorPath Parse(string path) /// TBD public static bool TryParse(string path, out ActorPath actorPath) { + //todo lookup address and/or root in cache + if (!TryParseAddress(path, out var address, out var spanified)) { actorPath = null; @@ -412,7 +414,8 @@ public static bool TryParse(string path, out ActorPath actorPath) nextSlash = spanified.IndexOf('/'); if (nextSlash > 0) { - actorPath /= spanified.Slice(0, nextSlash).ToString(); + var name = spanified.Slice(0, nextSlash).ToString(); + actorPath = new ChildActorPath(actorPath, name, ActorCell.UndefinedUid); } else if (nextSlash < 0 && spanified.Length > 0) // final segment { @@ -426,13 +429,14 @@ public static bool TryParse(string path, out ActorPath actorPath) } else { - actorPath /= spanified.ToString(); + actorPath = new ChildActorPath(actorPath, spanified.ToString(), ActorCell.UndefinedUid); } } spanified = spanified.Slice(nextSlash + 1); - } while (nextSlash >= 0); + } + while (nextSlash >= 0); return true; } @@ -480,7 +484,8 @@ private static bool TryParseAddress(string path, out Address address, out ReadOn string sysName; if (firstAtPos == -1) - { // dealing with an absolute local Uri + { + // dealing with an absolute local Uri var nextSlash = spanified.IndexOf('/'); if (nextSlash == -1) @@ -572,36 +577,45 @@ private static bool TryParseAddress(string path, out Address address, out ReadOn /// /// Joins this instance. /// + /// the address or empty /// System.String. - private string Join() + private string Join(ReadOnlySpan prefix) { - if (this is RootActorPath) - return "/"; - - // Resolve length of final string - var totalLength = 0; - var p = this; - while (!(p is RootActorPath)) + if (_depth == 0) { - totalLength += p.Name.Length + 1; - p = p.Parent; + Span buffer = stackalloc char[prefix.Length+1]; + prefix.CopyTo(buffer); + buffer[buffer.Length-1] = '/'; + return buffer.ToString(); } - - // Concatenate segments (in reverse order) into buffer with '/' prefixes - char[] buffer = new char[totalLength]; - int offset = buffer.Length; - p = this; - while (!(p is RootActorPath)) + else { - offset -= p.Name.Length + 1; - buffer[offset] = '/'; + // Resolve length of final string + var totalLength = prefix.Length; + var p = this; + while (p._depth > 0) + { + totalLength += p._name.Length + 1; + p = p.Parent; + } - p.Name.CopyTo(0, buffer, offset + 1, p.Name.Length); + // Concatenate segments (in reverse order) into buffer with '/' prefixes + Span buffer = stackalloc char[totalLength]; + prefix.CopyTo(buffer); - p = p.Parent; - } - - return new string(buffer); + var offset = buffer.Length; + ReadOnlySpan name; + p = this; + while (p._depth > 0) + { + name = p._name.AsSpan(); + offset -= name.Length + 1; + buffer[offset] = '/'; + name.CopyTo(buffer.Slice(offset + 1, name.Length)); + p = p.Parent; + } + return buffer.ToString(); + } } /// @@ -612,13 +626,13 @@ private string Join() /// System.String. public string ToStringWithoutAddress() { - return Join(); + return Join(ReadOnlySpan.Empty); } /// public override string ToString() { - return $"{Address}{Join()}"; + return Join(_address.ToString().AsSpan()); } /// @@ -627,7 +641,7 @@ public override string ToString() /// TBD public string ToStringWithUid() { - return Uid != ActorCell.UndefinedUid ? $"{ToStringWithAddress()}#{Uid}" : ToStringWithAddress(); + return _uid != ActorCell.UndefinedUid ? $"{ToStringWithAddress()}#{_uid}" : ToStringWithAddress(); } /// @@ -666,7 +680,7 @@ public override bool Equals(object obj) /// true if both actor paths are equal; otherwise false public static bool operator ==(ActorPath left, ActorPath right) { - return Equals(left, right); + return left?.Equals(right) ?? right is null; } /// @@ -677,7 +691,7 @@ public override bool Equals(object obj) /// true if both actor paths are not equal; otherwise false public static bool operator !=(ActorPath left, ActorPath right) { - return !Equals(left, right); + return !(left == right); } /// @@ -686,7 +700,7 @@ public override bool Equals(object obj) /// System.String. public string ToStringWithAddress() { - return ToStringWithAddress(Address); + return ToStringWithAddress(_address); } /// @@ -717,7 +731,7 @@ public string ToSerializationFormatWithAddress(Address address) private string AppendUidFragment(string withAddress) { - return Uid != ActorCell.UndefinedUid ? $"{withAddress}#{Uid}" : withAddress; + return _uid != ActorCell.UndefinedUid ? $"{withAddress}#{_uid}" : withAddress; } /// @@ -734,10 +748,10 @@ public string ToStringWithAddress(Address address) // we never change address for IgnoreActorRef return ToString(); } - if (Address.Host != null && Address.Port.HasValue) - return $"{Address}{Join()}"; + if (_address.Host != null && _address.Port.HasValue) + return Join(_address.ToString().AsSpan()); - return $"{address}{Join()}"; + return Join(address.ToString().AsSpan()); } /// diff --git a/src/core/Akka/Actor/Address.cs b/src/core/Akka/Actor/Address.cs index 81398855cc6..8b36bd5f38e 100644 --- a/src/core/Akka/Actor/Address.cs +++ b/src/core/Akka/Actor/Address.cs @@ -303,7 +303,7 @@ public static Address Parse(string address) /// This class represents a surrogate of an . /// Its main use is to help during the serialization process. /// - public class AddressSurrogate : ISurrogate + public sealed class AddressSurrogate : ISurrogate { /// /// TBD From aa211c7a0f7ca76c506afc34f39f0d1da4ea0e01 Mon Sep 17 00:00:00 2001 From: zetanova Date: Fri, 10 Sep 2021 03:15:03 +0200 Subject: [PATCH 11/33] improve actor path equals and compare --- src/core/Akka/Actor/ActorPath.cs | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/core/Akka/Actor/ActorPath.cs b/src/core/Akka/Actor/ActorPath.cs index 6e4bff6df77..608ee575bb3 100644 --- a/src/core/Akka/Actor/ActorPath.cs +++ b/src/core/Akka/Actor/ActorPath.cs @@ -273,25 +273,25 @@ public ActorPath Root /// public bool Equals(ActorPath other) { - if (other == null) + if (other is null || _depth != other._depth) return false; if (!Address.Equals(other.Address)) return false; - ActorPath a = this; - ActorPath b = other; + var a = this; + var b = other; while (true) { if (ReferenceEquals(a, b)) return true; - else if (a == null || b == null) + else if (a is null || b is null) return false; - else if (a.Name != b.Name) + else if (a._name != b._name) return false; - a = a.Parent; - b = b.Parent; + a = a._parent; + b = b._parent; } } @@ -300,8 +300,8 @@ public int CompareTo(ActorPath other) { if (_depth == 0) { - if (other is ChildActorPath) return 1; - return StringComparer.Ordinal.Compare(ToString(), other?.ToString()); + if (other is null || other._depth > 0) return 1; + return StringComparer.Ordinal.Compare(ToString(), other.ToString()); } return InternalCompareTo(this, other); } @@ -310,6 +310,10 @@ private int InternalCompareTo(ActorPath left, ActorPath right) { if (ReferenceEquals(left, right)) return 0; + if (right is null) + return 1; + if (left is null) + return -1; if (left._depth == 0) return left.CompareTo(right); From 1842505e6a399536f9ecd26c43a5dbb4b8a9b966 Mon Sep 17 00:00:00 2001 From: zetanova Date: Fri, 10 Sep 2021 03:26:29 +0200 Subject: [PATCH 12/33] cleanup --- src/core/Akka/Actor/ActorPath.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/core/Akka/Actor/ActorPath.cs b/src/core/Akka/Actor/ActorPath.cs index 608ee575bb3..82dc3aed100 100644 --- a/src/core/Akka/Actor/ActorPath.cs +++ b/src/core/Akka/Actor/ActorPath.cs @@ -74,7 +74,7 @@ public bool Equals(Surrogate other) public bool Equals(ActorPath other) { if (other is null) return false; - return Equals(other.ToSurrogate(null)); //TODO: not so sure if this is OK + return StringComparer.Ordinal.Equals(Path, other.ToSerializationFormat()); } /// @@ -120,7 +120,7 @@ private static bool IsHexChar(char c) => (c >= 'a' && c <= 'f') || (c >= 'A' && private static bool Validate(string chars) { - int len = chars.Length; + var len = chars.Length; var pos = 0; while (pos < len) { @@ -341,7 +341,7 @@ public ActorPath WithUid(long uid) return this; } - return uid != Uid ? new ChildActorPath(_parent, Name, uid) : this; + return uid != _uid ? new ChildActorPath(_parent, Name, uid) : this; } /// @@ -367,10 +367,10 @@ public ActorPath WithUid(long uid) public static ActorPath operator /(ActorPath path, IEnumerable name) { var a = path; - foreach (string element in name) + foreach (var element in name) { if (!string.IsNullOrEmpty(element)) - a = a / element; + a /= element; } return a; } @@ -587,9 +587,9 @@ private string Join(ReadOnlySpan prefix) { if (_depth == 0) { - Span buffer = stackalloc char[prefix.Length+1]; + Span buffer = stackalloc char[prefix.Length + 1]; prefix.CopyTo(buffer); - buffer[buffer.Length-1] = '/'; + buffer[buffer.Length - 1] = '/'; return buffer.ToString(); } else @@ -619,7 +619,7 @@ private string Join(ReadOnlySpan prefix) p = p.Parent; } return buffer.ToString(); - } + } } /// From f388ceff66519c95db5723013f55f1735f513797 Mon Sep 17 00:00:00 2001 From: zetanova Date: Sat, 11 Sep 2021 21:27:05 +0200 Subject: [PATCH 13/33] protect stack and use moveto of arraybuilder --- src/core/Akka/Actor/ActorPath.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/Akka/Actor/ActorPath.cs b/src/core/Akka/Actor/ActorPath.cs index 82dc3aed100..7c4664c8157 100644 --- a/src/core/Akka/Actor/ActorPath.cs +++ b/src/core/Akka/Actor/ActorPath.cs @@ -224,7 +224,7 @@ public IReadOnlyList Elements b[_depth - i - 1] = p.Name; p = p._parent; } - return b.ToImmutable(); + return b.MoveToImmutable(); } } @@ -251,7 +251,7 @@ internal IReadOnlyList ElementsWithUid b[_depth - i - 1] = i > 0 ? p._name : AppendUidFragment(p._name); p = p._parent; } - return b.ToImmutable(); + return b.MoveToImmutable(); } } @@ -587,7 +587,7 @@ private string Join(ReadOnlySpan prefix) { if (_depth == 0) { - Span buffer = stackalloc char[prefix.Length + 1]; + Span buffer = prefix.Length < 1024 ? stackalloc char[prefix.Length + 1] : new char[prefix.Length + 1]; prefix.CopyTo(buffer); buffer[buffer.Length - 1] = '/'; return buffer.ToString(); @@ -604,7 +604,7 @@ private string Join(ReadOnlySpan prefix) } // Concatenate segments (in reverse order) into buffer with '/' prefixes - Span buffer = stackalloc char[totalLength]; + Span buffer = totalLength < 1024 ? stackalloc char[totalLength] : new char[totalLength]; prefix.CopyTo(buffer); var offset = buffer.Length; From baaeea2771d4381ce3f677a46a94643bd5a8eba0 Mon Sep 17 00:00:00 2001 From: zetanova Date: Sat, 11 Sep 2021 21:34:05 +0200 Subject: [PATCH 14/33] update api spec --- .../CoreAPISpec.ApproveCore.approved.txt | 32 +++++++------------ 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt b/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt index 32a68ea2cd0..a859abc0411 100644 --- a/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt +++ b/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt @@ -177,13 +177,15 @@ namespace Akka.Actor protected ActorPath(Akka.Actor.Address address, string name) { } protected ActorPath(Akka.Actor.ActorPath parentPath, string name, long uid) { } public Akka.Actor.Address Address { get; } - public abstract System.Collections.Generic.IReadOnlyList Elements { get; } + public int Depth { get; } + public System.Collections.Generic.IReadOnlyList Elements { get; } public string Name { get; } - public abstract Akka.Actor.ActorPath Parent { get; } - public abstract Akka.Actor.ActorPath Root { get; } + public Akka.Actor.ActorPath Parent { get; } + [Newtonsoft.Json.JsonIgnoreAttribute()] + public Akka.Actor.ActorPath Root { get; } public long Uid { get; } public Akka.Actor.ActorPath Child(string childName) { } - public abstract int CompareTo(Akka.Actor.ActorPath other); + public int CompareTo(Akka.Actor.ActorPath other) { } public bool Equals(Akka.Actor.ActorPath other) { } public override bool Equals(object obj) { } public static string FormatPathElements(System.Collections.Generic.IEnumerable pathElements) { } @@ -200,12 +202,12 @@ namespace Akka.Actor public Akka.Util.ISurrogate ToSurrogate(Akka.Actor.ActorSystem system) { } public static bool TryParse(string path, out Akka.Actor.ActorPath actorPath) { } public static bool TryParseAddress(string path, out Akka.Actor.Address address) { } - public abstract Akka.Actor.ActorPath WithUid(long uid); + public Akka.Actor.ActorPath WithUid(long uid) { } public static Akka.Actor.ActorPath /(Akka.Actor.ActorPath path, string name) { } public static Akka.Actor.ActorPath /(Akka.Actor.ActorPath path, System.Collections.Generic.IEnumerable name) { } public static bool ==(Akka.Actor.ActorPath left, Akka.Actor.ActorPath right) { } public static bool !=(Akka.Actor.ActorPath left, Akka.Actor.ActorPath right) { } - public class Surrogate : Akka.Util.ISurrogate, System.IEquatable, System.IEquatable + public sealed class Surrogate : Akka.Util.ISurrogate, System.IEquatable, System.IEquatable { public Surrogate(string path) { } public string Path { get; } @@ -414,7 +416,7 @@ namespace Akka.Actor public Akka.Actor.Address WithSystem(string system) { } public static bool ==(Akka.Actor.Address left, Akka.Actor.Address right) { } public static bool !=(Akka.Actor.Address left, Akka.Actor.Address right) { } - public class AddressSurrogate : Akka.Util.ISurrogate + public sealed class AddressSurrogate : Akka.Util.ISurrogate { public AddressSurrogate() { } public string Host { get; set; } @@ -497,15 +499,9 @@ namespace Akka.Actor { public static void CancelIfNotNull(this Akka.Actor.ICancelable cancelable) { } } - public class ChildActorPath : Akka.Actor.ActorPath + public sealed class ChildActorPath : Akka.Actor.ActorPath { public ChildActorPath(Akka.Actor.ActorPath parentPath, string name, long uid) { } - public override System.Collections.Generic.IReadOnlyList Elements { get; } - public override Akka.Actor.ActorPath Parent { get; } - public override Akka.Actor.ActorPath Root { get; } - public override int CompareTo(Akka.Actor.ActorPath other) { } - public override int GetHashCode() { } - public override Akka.Actor.ActorPath WithUid(long uid) { } } public sealed class CoordinatedShutdown : Akka.Actor.IExtension { @@ -1544,15 +1540,9 @@ namespace Akka.Actor public void SwapUnderlying(Akka.Actor.ICell cell) { } protected override void TellInternal(object message, Akka.Actor.IActorRef sender) { } } - public class RootActorPath : Akka.Actor.ActorPath + public sealed class RootActorPath : Akka.Actor.ActorPath { public RootActorPath(Akka.Actor.Address address, string name = "") { } - public override System.Collections.Generic.IReadOnlyList Elements { get; } - public override Akka.Actor.ActorPath Parent { get; } - [Newtonsoft.Json.JsonIgnoreAttribute()] - public override Akka.Actor.ActorPath Root { get; } - public override int CompareTo(Akka.Actor.ActorPath other) { } - public override Akka.Actor.ActorPath WithUid(long uid) { } } [Akka.Annotations.InternalApiAttribute()] public class RootGuardianActorRef : Akka.Actor.LocalActorRef From f86dc4bcd8e6c6e73c025dcc25d69afb02d9019a Mon Sep 17 00:00:00 2001 From: zetanova Date: Sat, 11 Sep 2021 22:07:43 +0200 Subject: [PATCH 15/33] test for jumbo actor path name support --- src/core/Akka.Tests/Actor/ActorPathSpec.cs | 17 +++++++++++++++++ src/core/Akka/Actor/ActorPath.cs | 6 +++--- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/core/Akka.Tests/Actor/ActorPathSpec.cs b/src/core/Akka.Tests/Actor/ActorPathSpec.cs index 31d51313842..f93c3bfebd2 100644 --- a/src/core/Akka.Tests/Actor/ActorPathSpec.cs +++ b/src/core/Akka.Tests/Actor/ActorPathSpec.cs @@ -111,6 +111,23 @@ public void Return_false_upon_malformed_path() ActorPath.TryParse("akka:/", out _).ShouldBe(false); } + [Fact] + public void Supports_jumbo_actor_name_length() + { + ReadOnlySpan prefix = "akka://sys@host.domain.com:1234/some/ref/".AsSpan(); + Span b = new char[10 * 1024 * 1024]; //10 MB + prefix.CopyTo(b); + b.Slice(prefix.Length).Fill('a'); + var path = b.ToString(); + + ActorPath.TryParse(path, out var actorPath).ShouldBe(true); + actorPath.Name.Length.ShouldBe(b.Length - prefix.Length); + actorPath.Name.All(n => n == 'a').ShouldBe(true); + + var result = actorPath.ToStringWithAddress(); + result.AsSpan().SequenceEqual(b).ShouldBe(true); + } + [Fact] public void Create_correct_ToString() { diff --git a/src/core/Akka/Actor/ActorPath.cs b/src/core/Akka/Actor/ActorPath.cs index 7c4664c8157..e7328c3cf54 100644 --- a/src/core/Akka/Actor/ActorPath.cs +++ b/src/core/Akka/Actor/ActorPath.cs @@ -590,7 +590,7 @@ private string Join(ReadOnlySpan prefix) Span buffer = prefix.Length < 1024 ? stackalloc char[prefix.Length + 1] : new char[prefix.Length + 1]; prefix.CopyTo(buffer); buffer[buffer.Length - 1] = '/'; - return buffer.ToString(); + return buffer.ToString(); //todo use string.Create() when available } else { @@ -603,7 +603,7 @@ private string Join(ReadOnlySpan prefix) p = p.Parent; } - // Concatenate segments (in reverse order) into buffer with '/' prefixes + // Concatenate segments (in reverse order) into buffer with '/' prefixes Span buffer = totalLength < 1024 ? stackalloc char[totalLength] : new char[totalLength]; prefix.CopyTo(buffer); @@ -618,7 +618,7 @@ private string Join(ReadOnlySpan prefix) name.CopyTo(buffer.Slice(offset + 1, name.Length)); p = p.Parent; } - return buffer.ToString(); + return buffer.ToString(); //todo use string.Create() when available } } From 4427087b8bac0828002c43e7e42d4728c0bc9a2f Mon Sep 17 00:00:00 2001 From: zetanova Date: Sun, 12 Sep 2021 23:23:39 +0200 Subject: [PATCH 16/33] small refactors --- .../Akka.Remote/RemoteActorRefProvider.cs | 12 +++++------ .../Serialization/ActorPathCache.cs | 5 ++--- src/core/Akka/Actor/ActorPath.cs | 3 ++- src/core/Akka/Actor/ActorRefFactoryShared.cs | 3 +-- src/core/Akka/Actor/ActorRefProvider.cs | 4 ++-- src/core/Akka/Actor/ActorSelection.cs | 21 ++++++++----------- src/core/Akka/Actor/Futures.cs | 12 +++++------ 7 files changed, 28 insertions(+), 32 deletions(-) diff --git a/src/core/Akka.Remote/RemoteActorRefProvider.cs b/src/core/Akka.Remote/RemoteActorRefProvider.cs index 325adacdbda..32918533503 100644 --- a/src/core/Akka.Remote/RemoteActorRefProvider.cs +++ b/src/core/Akka.Remote/RemoteActorRefProvider.cs @@ -469,6 +469,12 @@ private IInternalActorRef LocalActorOf(ActorSystemImpl system, Props props, IInt /// TBD public IInternalActorRef ResolveActorRefWithLocalAddress(string path, Address localAddress) { + if (path is null) + { + _log.Debug("resolve of unknown path [{0}] failed", path); + return InternalDeadLetters; + } + ActorPath actorPath; if (_actorPathThreadLocalCache != null) { @@ -479,12 +485,6 @@ public IInternalActorRef ResolveActorRefWithLocalAddress(string path, Address lo ActorPath.TryParse(path, out actorPath); } - if (path is null) - { - _log.Debug("resolve of unknown path [{0}] failed", path); - return InternalDeadLetters; - } - if (!HasAddress(actorPath.Address)) return CreateRemoteRef(new RootActorPath(actorPath.Address) / actorPath.ElementsWithUid, localAddress); diff --git a/src/core/Akka.Remote/Serialization/ActorPathCache.cs b/src/core/Akka.Remote/Serialization/ActorPathCache.cs index 1f292dcb1df..c8c6e82bac8 100644 --- a/src/core/Akka.Remote/Serialization/ActorPathCache.cs +++ b/src/core/Akka.Remote/Serialization/ActorPathCache.cs @@ -50,9 +50,8 @@ protected override int Hash(string k) protected override ActorPath Compute(string k) { - if (ActorPath.TryParse(k, out var actorPath)) - return actorPath; - return null; + ActorPath.TryParse(k, out var actorPath); + return actorPath; } protected override bool IsCacheable(ActorPath v) diff --git a/src/core/Akka/Actor/ActorPath.cs b/src/core/Akka/Actor/ActorPath.cs index e7328c3cf54..bedd9e9c7a5 100644 --- a/src/core/Akka/Actor/ActorPath.cs +++ b/src/core/Akka/Actor/ActorPath.cs @@ -58,7 +58,8 @@ public Surrogate(string path) /// The encapsulated by this surrogate. public ISurrogated FromSurrogate(ActorSystem system) { - return TryParse(Path, out var path) ? path : null; + TryParse(Path, out var path); + return path; } #region Equality diff --git a/src/core/Akka/Actor/ActorRefFactoryShared.cs b/src/core/Akka/Actor/ActorRefFactoryShared.cs index 032b263a3dc..067f62586aa 100644 --- a/src/core/Akka/Actor/ActorRefFactoryShared.cs +++ b/src/core/Akka/Actor/ActorRefFactoryShared.cs @@ -64,8 +64,7 @@ public static ActorSelection ActorSelection(string path, ActorSystem system, IAc if(Uri.IsWellFormedUriString(path, UriKind.Absolute)) { - ActorPath actorPath; - if(!ActorPath.TryParse(path, out actorPath)) + if(!ActorPath.TryParse(path, out var actorPath)) return new ActorSelection(provider.DeadLetters, ""); var actorRef = provider.RootGuardianAt(actorPath.Address); diff --git a/src/core/Akka/Actor/ActorRefProvider.cs b/src/core/Akka/Actor/ActorRefProvider.cs index aa6e21adb95..efd4c77ef26 100644 --- a/src/core/Akka/Actor/ActorRefProvider.cs +++ b/src/core/Akka/Actor/ActorRefProvider.cs @@ -417,9 +417,9 @@ public void Init(ActorSystemImpl system) /// TBD public IActorRef ResolveActorRef(string path) { - ActorPath actorPath; - if (ActorPath.TryParse(path, out actorPath) && actorPath.Address == _rootPath.Address) + if (ActorPath.TryParse(path, out var actorPath) && actorPath.Address == _rootPath.Address) return ResolveActorRef(_rootGuardian, actorPath.Elements); + _log.Debug("Resolve of unknown path [{0}] failed. Invalid format.", path); return _deadLetters; } diff --git a/src/core/Akka/Actor/ActorSelection.cs b/src/core/Akka/Actor/ActorSelection.cs index 218b3c2838f..a13dcca96e5 100644 --- a/src/core/Akka/Actor/ActorSelection.cs +++ b/src/core/Akka/Actor/ActorSelection.cs @@ -63,7 +63,7 @@ public ActorSelection(IActorRef anchor, SelectionPathElement[] path) /// The anchor. /// The path. public ActorSelection(IActorRef anchor, string path) - : this(anchor, path == "" ? new string[] { } : path.Split('/')) + : this(anchor, path == "" ? Array.Empty() : path.Split('/')) { } @@ -77,8 +77,8 @@ public ActorSelection(IActorRef anchor, IEnumerable elements) Anchor = anchor; var list = new List(); - var count = elements.Count(); // shouldn't have a multiple enumeration issue\ - var i = 0; + var hasDoubleWildcard = false; + foreach (var s in elements) { switch (s) @@ -86,10 +86,9 @@ public ActorSelection(IActorRef anchor, IEnumerable elements) case null: case "": break; - case "**": - if (i < count-1) - throw new IllegalActorNameException("Double wildcard can only appear at the last path entry"); + case "**": list.Add(SelectChildRecursive.Instance); + hasDoubleWildcard = true; break; case string e when e.Contains("?") || e.Contains("*"): list.Add(new SelectChildPattern(e)); @@ -101,10 +100,11 @@ public ActorSelection(IActorRef anchor, IEnumerable elements) list.Add(new SelectChildName(s)); break; } - - i++; } + if(hasDoubleWildcard && list[list.Count-1] != SelectChildRecursive.Instance) + throw new IllegalActorNameException("Double wildcard can only appear at the last path entry"); + Path = list.ToArray(); } @@ -164,10 +164,7 @@ private async Task InnerResolveOne(TimeSpan timeout, CancellationToke try { var identity = await this.Ask(new Identify(null), timeout, ct).ConfigureAwait(false); - if (identity.Subject == null) - throw new ActorNotFoundException("subject was null"); - - return identity.Subject; + return identity.Subject ?? throw new ActorNotFoundException("subject was null"); } catch (Exception ex) { diff --git a/src/core/Akka/Actor/Futures.cs b/src/core/Akka/Actor/Futures.cs index c49cc8b74e4..fbb84507bfb 100644 --- a/src/core/Akka/Actor/Futures.cs +++ b/src/core/Akka/Actor/Futures.cs @@ -178,14 +178,14 @@ public static Task Ask(this ICanTell self, Func message /// Provider used for Ask pattern implementation internal static IActorRefProvider ResolveProvider(ICanTell self) { - if (self is ActorSelection) - return ResolveProvider(self.AsInstanceOf().Anchor); + if (self is ActorSelection selection) + return ResolveProvider(selection.Anchor); - if (self is IInternalActorRef) - return self.AsInstanceOf().Provider; + if (self is IInternalActorRef actorRef) + return actorRef.Provider; - if (ActorCell.Current != null) - return InternalCurrentActorCellKeeper.Current.SystemImpl.Provider; + if (ActorCell.Current is ActorCell cell) + return cell.SystemImpl.Provider; return null; } From 93f5d9c6a6b360c081916fa64d1d538001678df3 Mon Sep 17 00:00:00 2001 From: zetanova Date: Mon, 13 Sep 2021 00:05:08 +0200 Subject: [PATCH 17/33] add ActorPath.ParentOf(depth) --- .../CoreAPISpec.ApproveCore.approved.txt | 1 + src/core/Akka/Actor/ActorPath.cs | 32 +++++++++++++------ 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt b/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt index a859abc0411..d160abede3d 100644 --- a/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt +++ b/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt @@ -191,6 +191,7 @@ namespace Akka.Actor public static string FormatPathElements(System.Collections.Generic.IEnumerable pathElements) { } public override int GetHashCode() { } public static bool IsValidPathElement(string s) { } + public Akka.Actor.ActorPath ParentOf(int depth) { } public static Akka.Actor.ActorPath Parse(string path) { } public string ToSerializationFormat() { } public string ToSerializationFormatWithAddress(Akka.Actor.Address address) { } diff --git a/src/core/Akka/Actor/ActorPath.cs b/src/core/Akka/Actor/ActorPath.cs index bedd9e9c7a5..6931fdce357 100644 --- a/src/core/Akka/Actor/ActorPath.cs +++ b/src/core/Akka/Actor/ActorPath.cs @@ -260,16 +260,7 @@ internal IReadOnlyList ElementsWithUid /// The root actor path. /// [JsonIgnore] - public ActorPath Root - { - get - { - var current = this; - while (current._depth > 0) - current = current.Parent; - return current; - } - } + public ActorPath Root => ParentOf(0); /// public bool Equals(ActorPath other) @@ -376,6 +367,27 @@ public ActorPath WithUid(long uid) return a; } + /// + /// Returns a parent of depth + /// 0: Root, 1: Guardian, -1: Parent, -2: GrandParent + /// + /// The parent depth, negative depth for reverse lookup + public ActorPath ParentOf(int depth) + { + var current = this; + if (depth >= 0) + { + while (current._depth > depth) + current = current.Parent; + } + else + { + for(var i = depth; i < 0 && current.Depth > 0; i++) + current = current.Parent; + } + return current; + } + /// /// Creates an from the specified . /// From d74deaad5b54df6b46c576f82810a0fee95f8edd Mon Sep 17 00:00:00 2001 From: zetanova Date: Mon, 13 Sep 2021 00:20:16 +0200 Subject: [PATCH 18/33] dont copy actorpath --- src/core/Akka.Remote/RemoteActorRefProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/Akka.Remote/RemoteActorRefProvider.cs b/src/core/Akka.Remote/RemoteActorRefProvider.cs index 32918533503..f3165c98791 100644 --- a/src/core/Akka.Remote/RemoteActorRefProvider.cs +++ b/src/core/Akka.Remote/RemoteActorRefProvider.cs @@ -486,7 +486,7 @@ public IInternalActorRef ResolveActorRefWithLocalAddress(string path, Address lo } if (!HasAddress(actorPath.Address)) - return CreateRemoteRef(new RootActorPath(actorPath.Address) / actorPath.ElementsWithUid, localAddress); + return CreateRemoteRef(actorPath, localAddress); //the actor's local address was already included in the ActorPath From a7a525e48ff391d80d592ff3a4a1a5442f9ae180 Mon Sep 17 00:00:00 2001 From: zetanova Date: Mon, 13 Sep 2021 00:28:42 +0200 Subject: [PATCH 19/33] use actorpath-cache and remove cache entry test --- src/core/Akka.Remote.Tests/RemotingSpec.cs | 54 +++++++++---------- .../Serialization/ActorPathCache.cs | 4 +- .../Serialization/ActorRefResolveCache.cs | 4 +- .../Akka.Remote/Transport/AkkaPduCodec.cs | 6 +-- 4 files changed, 32 insertions(+), 36 deletions(-) diff --git a/src/core/Akka.Remote.Tests/RemotingSpec.cs b/src/core/Akka.Remote.Tests/RemotingSpec.cs index 7618698411e..876740461ce 100644 --- a/src/core/Akka.Remote.Tests/RemotingSpec.cs +++ b/src/core/Akka.Remote.Tests/RemotingSpec.cs @@ -178,33 +178,33 @@ public async Task Remoting_must_support_Ask() Assert.IsType>(actorRef); } - [Fact] - public async Task Remoting_should_not_cache_ref_of_local_ask() - { - var localActorRefResolveCache = ActorRefResolveThreadLocalCache.For(Sys); - var localActorPathCache = ActorPathThreadLocalCache.For(Sys); - - var (msg, actorRef) = await _here.Ask<(string, IActorRef)>("ping", DefaultTimeout); - Assert.Equal("pong", msg); - Assert.IsType>(actorRef); - - Assert.Equal(0, localActorRefResolveCache.All.Sum(n => n.Stats.Entries)); - Assert.Equal(2, localActorPathCache.All.Sum(n => n.Stats.Entries)); - } - - [Fact] - public async Task Remoting_should_not_cache_ref_of_remote_ask() - { - var remoteActorRefResolveCache = ActorRefResolveThreadLocalCache.For(_remoteSystem); - var remoteActorPathCache = ActorPathThreadLocalCache.For(_remoteSystem); - - var (msg, actorRef) = await _here.Ask<(string, IActorRef)>("ping", DefaultTimeout); - Assert.Equal("pong", msg); - Assert.IsType>(actorRef); - - Assert.Equal(0, remoteActorRefResolveCache.All.Sum(n => n.Stats.Entries)); - Assert.Equal(2, remoteActorPathCache.All.Sum(n => n.Stats.Entries)); //should be 1 - } + //[Fact] + //public async Task Remoting_should_not_cache_ref_of_local_ask() + //{ + // var localActorRefResolveCache = ActorRefResolveThreadLocalCache.For(Sys); + // var localActorPathCache = ActorPathThreadLocalCache.For(Sys); + + // var (msg, actorRef) = await _here.Ask<(string, IActorRef)>("ping", DefaultTimeout); + // Assert.Equal("pong", msg); + // Assert.IsType>(actorRef); + + // Assert.Equal(0, localActorRefResolveCache.All.Sum(n => n.Stats.Entries)); + // Assert.Equal(2, localActorPathCache.All.Sum(n => n.Stats.Entries)); + //} + + //[Fact] + //public async Task Remoting_should_not_cache_ref_of_remote_ask() + //{ + // var remoteActorRefResolveCache = ActorRefResolveThreadLocalCache.For(_remoteSystem); + // var remoteActorPathCache = ActorPathThreadLocalCache.For(_remoteSystem); + + // var (msg, actorRef) = await _here.Ask<(string, IActorRef)>("ping", DefaultTimeout); + // Assert.Equal("pong", msg); + // Assert.IsType>(actorRef); + + // Assert.Equal(0, remoteActorRefResolveCache.All.Sum(n => n.Stats.Entries)); + // Assert.Equal(2, remoteActorPathCache.All.Sum(n => n.Stats.Entries)); //should be 1 + //} [Fact(Skip = "Racy")] public async Task Ask_does_not_deadlock() diff --git a/src/core/Akka.Remote/Serialization/ActorPathCache.cs b/src/core/Akka.Remote/Serialization/ActorPathCache.cs index c8c6e82bac8..e3454e1065a 100644 --- a/src/core/Akka.Remote/Serialization/ActorPathCache.cs +++ b/src/core/Akka.Remote/Serialization/ActorPathCache.cs @@ -17,12 +17,10 @@ namespace Akka.Remote.Serialization /// internal sealed class ActorPathThreadLocalCache : ExtensionIdProvider, IExtension { - private readonly ThreadLocal _current = new ThreadLocal(() => new ActorPathCache(), true); + private readonly ThreadLocal _current = new ThreadLocal(() => new ActorPathCache(), false); public ActorPathCache Cache => _current.Value; - internal IList All => _current.Values; - public override ActorPathThreadLocalCache CreateExtension(ExtendedActorSystem system) { return new ActorPathThreadLocalCache(); diff --git a/src/core/Akka.Remote/Serialization/ActorRefResolveCache.cs b/src/core/Akka.Remote/Serialization/ActorRefResolveCache.cs index 4b650b6e894..e0bebe1de28 100644 --- a/src/core/Akka.Remote/Serialization/ActorRefResolveCache.cs +++ b/src/core/Akka.Remote/Serialization/ActorRefResolveCache.cs @@ -24,7 +24,7 @@ public ActorRefResolveThreadLocalCache() { } public ActorRefResolveThreadLocalCache(IRemoteActorRefProvider provider) { _provider = provider; - _current = new ThreadLocal(() => new ActorRefResolveCache(_provider), true); + _current = new ThreadLocal(() => new ActorRefResolveCache(_provider), false); } public override ActorRefResolveThreadLocalCache CreateExtension(ExtendedActorSystem system) @@ -36,8 +36,6 @@ public override ActorRefResolveThreadLocalCache CreateExtension(ExtendedActorSys public ActorRefResolveCache Cache => _current.Value; - internal IList All => _current.Values; - public static ActorRefResolveThreadLocalCache For(ActorSystem system) { return system.WithExtension(); diff --git a/src/core/Akka.Remote/Transport/AkkaPduCodec.cs b/src/core/Akka.Remote/Transport/AkkaPduCodec.cs index 8e257d47a55..335483e5ffb 100644 --- a/src/core/Akka.Remote/Transport/AkkaPduCodec.cs +++ b/src/core/Akka.Remote/Transport/AkkaPduCodec.cs @@ -202,12 +202,12 @@ public AckAndMessage(Ack ackOption, Message messageOption) internal abstract class AkkaPduCodec { protected readonly ActorSystem System; - protected readonly AddressThreadLocalCache ActorPathCache; + protected readonly ActorPathThreadLocalCache ActorPathCache; protected AkkaPduCodec(ActorSystem system) { System = system; - ActorPathCache = AddressThreadLocalCache.For(system); + ActorPathCache = ActorPathThreadLocalCache.For(system); } /// @@ -428,7 +428,7 @@ public override AckAndMessage DecodeMessage(ByteString raw, IRemoteActorRefProvi var recipient = provider.ResolveActorRefWithLocalAddress(envelopeContainer.Recipient.Path, localAddress); //todo get parsed address from provider - var recipientAddress = ActorPathCache.Cache.GetOrCompute(envelopeContainer.Recipient.Path); + var recipientAddress = ActorPathCache.Cache.GetOrCompute(envelopeContainer.Recipient.Path).Address; var serializedMessage = envelopeContainer.Message; IActorRef senderOption = null; From d3c33e86d754e4ac38c5bb63b65f3641722a19c7 Mon Sep 17 00:00:00 2001 From: zetanova Date: Mon, 13 Sep 2021 01:04:17 +0200 Subject: [PATCH 20/33] refactor fill array --- src/core/Akka.Remote/Serialization/LruBoundedCache.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/Akka.Remote/Serialization/LruBoundedCache.cs b/src/core/Akka.Remote/Serialization/LruBoundedCache.cs index e2a99fde2f3..95f34a486a0 100644 --- a/src/core/Akka.Remote/Serialization/LruBoundedCache.cs +++ b/src/core/Akka.Remote/Serialization/LruBoundedCache.cs @@ -132,7 +132,8 @@ protected LruBoundedCache(int capacity, int evictAgeThreshold) _keys = new TKey[Capacity]; _values = new TValue[Capacity]; _hashes = new int[Capacity]; - _epochs = Enumerable.Repeat(_epoch - evictAgeThreshold, Capacity).ToArray(); + _epochs = new int[Capacity]; + _epochs.AsSpan().Fill(_epoch - evictAgeThreshold); } public int Capacity { get; private set; } From 4512aec1c6a59dd7be4edfe3ed7c55bf3abcc619 Mon Sep 17 00:00:00 2001 From: zetanova Date: Mon, 13 Sep 2021 01:38:55 +0200 Subject: [PATCH 21/33] prepair actor path cache for better deduplication --- .../Serialization/ActorPathCache.cs | 11 +++- .../Serialization/LruBoundedCache.cs | 20 ++++--- src/core/Akka/Actor/ActorPath.cs | 52 +++++++++++-------- 3 files changed, 55 insertions(+), 28 deletions(-) diff --git a/src/core/Akka.Remote/Serialization/ActorPathCache.cs b/src/core/Akka.Remote/Serialization/ActorPathCache.cs index e3454e1065a..d2bef0d7594 100644 --- a/src/core/Akka.Remote/Serialization/ActorPathCache.cs +++ b/src/core/Akka.Remote/Serialization/ActorPathCache.cs @@ -48,7 +48,16 @@ protected override int Hash(string k) protected override ActorPath Compute(string k) { - ActorPath.TryParse(k, out var actorPath); + //todo lookup in address cache + + if (!ActorPath.TryParseAddress(k, out var address, out var absoluteUri)) + return null; + + //todo lookup in root in cache + + if (!ActorPath.TryParse(new RootActorPath(address), absoluteUri, out var actorPath)) + return null; + return actorPath; } diff --git a/src/core/Akka.Remote/Serialization/LruBoundedCache.cs b/src/core/Akka.Remote/Serialization/LruBoundedCache.cs index 95f34a486a0..20819f85937 100644 --- a/src/core/Akka.Remote/Serialization/LruBoundedCache.cs +++ b/src/core/Akka.Remote/Serialization/LruBoundedCache.cs @@ -197,6 +197,12 @@ public TValue Get(TKey k) } public TValue GetOrCompute(TKey k) + { + TryGetOrCompute(k, out var value); + return value; + } + + public bool TryGetOrCompute(TKey k, out TValue value) { var h = Hash(k); unchecked { _epoch += 1; } @@ -208,7 +214,7 @@ public TValue GetOrCompute(TKey k) { if (_values[position] == null) { - var value = Compute(k); + value = Compute(k); if (IsCacheable(value)) { _keys[position] = k; @@ -216,7 +222,7 @@ public TValue GetOrCompute(TKey k) _hashes[position] = h; _epochs[position] = _epoch; } - return value; + return false; } else { @@ -225,15 +231,17 @@ public TValue GetOrCompute(TKey k) // the table since because of the Robin-Hood property we would have swapped it with the current element. if (probeDistance > otherProbeDistance) { - var value = Compute(k); - if (IsCacheable(value)) Move(position, k, h, value, _epoch, probeDistance); - return value; + value = Compute(k); + if (IsCacheable(value)) + Move(position, k, h, value, _epoch, probeDistance); + return false; } else if (_hashes[position] == h && k.Equals(_keys[position])) { // Update usage _epochs[position] = _epoch; - return _values[position]; + value = _values[position]; + return false; } else { diff --git a/src/core/Akka/Actor/ActorPath.cs b/src/core/Akka/Actor/ActorPath.cs index 6931fdce357..aa7df64ca8c 100644 --- a/src/core/Akka/Actor/ActorPath.cs +++ b/src/core/Akka/Actor/ActorPath.cs @@ -379,10 +379,10 @@ public ActorPath ParentOf(int depth) { while (current._depth > depth) current = current.Parent; - } + } else { - for(var i = depth; i < 0 && current.Depth > 0; i++) + for (var i = depth; i < 0 && current.Depth > 0; i++) current = current.Parent; } return current; @@ -398,10 +398,9 @@ public ActorPath ParentOf(int depth) /// A newly created public static ActorPath Parse(string path) { - if (!TryParse(path, out var actorPath)) - throw new UriFormatException($"Can not parse an ActorPath: {path}"); - - return actorPath; + return TryParse(path, out var actorPath) + ? actorPath + : throw new UriFormatException($"Can not parse an ActorPath: {path}"); } /// @@ -413,45 +412,56 @@ public static ActorPath Parse(string path) /// TBD public static bool TryParse(string path, out ActorPath actorPath) { - //todo lookup address and/or root in cache - - if (!TryParseAddress(path, out var address, out var spanified)) + if (!TryParseAddress(path, out var address, out var absoluteUri)) { actorPath = null; return false; } + return TryParse(new RootActorPath(address), absoluteUri, out actorPath); + } + + /// + /// Tries to parse the uri, which should be a uri not containing protocol. + /// For example "/user/my-actor" + /// + /// the base path, normaly a root path + /// TBD + /// TBD + /// TBD + public static bool TryParse(ActorPath basePath, ReadOnlySpan absoluteUri, out ActorPath actorPath) + { + actorPath = basePath; + // check for Uri fragment here int nextSlash; - actorPath = new RootActorPath(address); - do { - nextSlash = spanified.IndexOf('/'); + nextSlash = absoluteUri.IndexOf('/'); if (nextSlash > 0) { - var name = spanified.Slice(0, nextSlash).ToString(); + var name = absoluteUri.Slice(0, nextSlash).ToString(); actorPath = new ChildActorPath(actorPath, name, ActorCell.UndefinedUid); } - else if (nextSlash < 0 && spanified.Length > 0) // final segment + else if (nextSlash < 0 && absoluteUri.Length > 0) // final segment { - var fragLoc = spanified.IndexOf('#'); + var fragLoc = absoluteUri.IndexOf('#'); if (fragLoc > -1) { - var fragment = spanified.Slice(fragLoc + 1); + var fragment = absoluteUri.Slice(fragLoc + 1); var fragValue = SpanHacks.Parse(fragment); - spanified = spanified.Slice(0, fragLoc); - actorPath = new ChildActorPath(actorPath, spanified.ToString(), fragValue); + absoluteUri = absoluteUri.Slice(0, fragLoc); + actorPath = new ChildActorPath(actorPath, absoluteUri.ToString(), fragValue); } else { - actorPath = new ChildActorPath(actorPath, spanified.ToString(), ActorCell.UndefinedUid); + actorPath = new ChildActorPath(actorPath, absoluteUri.ToString(), ActorCell.UndefinedUid); } } - spanified = spanified.Slice(nextSlash + 1); + absoluteUri = absoluteUri.Slice(nextSlash + 1); } while (nextSlash >= 0); @@ -476,7 +486,7 @@ public static bool TryParseAddress(string path, out Address address) /// If true, the parsed . Otherwise null. /// A containing the path following the address. /// true if the could be parsed, false otherwise. - private static bool TryParseAddress(string path, out Address address, out ReadOnlySpan absoluteUri) + public static bool TryParseAddress(string path, out Address address, out ReadOnlySpan absoluteUri) { address = null; From ad3ca76ba02892739f617904baca5748d2f66467 Mon Sep 17 00:00:00 2001 From: zetanova Date: Mon, 13 Sep 2021 01:41:42 +0200 Subject: [PATCH 22/33] update api --- src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt b/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt index d160abede3d..a0b916f0397 100644 --- a/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt +++ b/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt @@ -202,7 +202,9 @@ namespace Akka.Actor public string ToStringWithoutAddress() { } public Akka.Util.ISurrogate ToSurrogate(Akka.Actor.ActorSystem system) { } public static bool TryParse(string path, out Akka.Actor.ActorPath actorPath) { } + public static bool TryParse(Akka.Actor.ActorPath basePath, System.ReadOnlySpan absoluteUri, out Akka.Actor.ActorPath actorPath) { } public static bool TryParseAddress(string path, out Akka.Actor.Address address) { } + public static bool TryParseAddress(string path, out Akka.Actor.Address address, out System.ReadOnlySpan absoluteUri) { } public Akka.Actor.ActorPath WithUid(long uid) { } public static Akka.Actor.ActorPath /(Akka.Actor.ActorPath path, string name) { } public static Akka.Actor.ActorPath /(Akka.Actor.ActorPath path, System.Collections.Generic.IEnumerable name) { } From 026a6fc6ae50dd57552281eb2da9a726f4404d54 Mon Sep 17 00:00:00 2001 From: zetanova Date: Wed, 15 Sep 2021 16:33:57 +0200 Subject: [PATCH 23/33] cache root actor path --- .../Serialization/LruBoundedCacheSpec.cs | 73 +++++++-- .../Serialization/ActorPathCache.cs | 54 +++++-- .../Serialization/ActorRefResolveCache.cs | 8 +- .../Akka.Remote/Serialization/AddressCache.cs | 8 +- .../Serialization/LruBoundedCache.cs | 144 ++++++++++++++++-- src/core/Akka/Actor/ActorPath.cs | 131 +++++----------- src/core/Akka/Actor/Address.cs | 95 +++++++++++- 7 files changed, 367 insertions(+), 146 deletions(-) diff --git a/src/core/Akka.Remote.Tests/Serialization/LruBoundedCacheSpec.cs b/src/core/Akka.Remote.Tests/Serialization/LruBoundedCacheSpec.cs index 9dc9eca751a..6b17e6bd9e1 100644 --- a/src/core/Akka.Remote.Tests/Serialization/LruBoundedCacheSpec.cs +++ b/src/core/Akka.Remote.Tests/Serialization/LruBoundedCacheSpec.cs @@ -5,6 +5,8 @@ // //----------------------------------------------------------------------- +using System; +using System.Collections.Generic; using System.Globalization; using System.Linq; using Akka.Remote.Serialization; @@ -14,22 +16,55 @@ namespace Akka.Remote.Tests.Serialization { + sealed class FastHashTestComparer : IEqualityComparer + { + private readonly string _hashSeed; + + public FastHashTestComparer(string hashSeed = "") + { + _hashSeed = hashSeed; + } + + public bool Equals(string x, string y) + { + return StringComparer.Ordinal.Equals(x, y); + } + + public int GetHashCode(string k) + { + return FastHash.OfStringFast(_hashSeed != string.Empty + ? _hashSeed + k + _hashSeed : k); + } + } + + sealed class BrokenTestComparer : IEqualityComparer + { + public bool Equals(string x, string y) + { + return StringComparer.Ordinal.Equals(x, y); + } + + public int GetHashCode(string k) + { + return 0; + } + } + public class LruBoundedCacheSpec { private class TestCache : LruBoundedCache { - public TestCache(int capacity, int evictAgeThreshold, string hashSeed = "") : base(capacity, evictAgeThreshold) + public TestCache(int capacity, int evictAgeThreshold, IEqualityComparer comparer) + : base(capacity, evictAgeThreshold, comparer) { - _hashSeed = hashSeed; } - private readonly string _hashSeed; - private int _cntr = 0; - - protected override int Hash(string k) + public TestCache(int capacity, int evictAgeThreshold, string hashSeed = "") + : base(capacity, evictAgeThreshold, new FastHashTestComparer(hashSeed)) { - return FastHash.OfStringFast(_hashSeed + k + _hashSeed); } + private int _cntr = 0; + protected override string Compute(string k) { var id = _cntr; @@ -71,20 +106,17 @@ public void ExpectComputedOnly(string key, string value) private sealed class BrokenHashFunctionTestCache : TestCache { - public BrokenHashFunctionTestCache(int capacity, int evictAgeThreshold, string hashSeed = "") : base(capacity, evictAgeThreshold, hashSeed) + public BrokenHashFunctionTestCache(int capacity, int evictAgeThreshold) : + base(capacity, evictAgeThreshold, new BrokenTestComparer()) { } - protected override int Hash(string k) - { - return 0; - } } [Fact] public void LruBoundedCache_must_work_in_the_happy_case() { - var cache = new TestCache(4,4); + var cache = new TestCache(4, 4); cache.ExpectComputed("A", "A:0"); cache.ExpectComputed("B", "B:1"); @@ -97,6 +129,19 @@ public void LruBoundedCache_must_work_in_the_happy_case() cache.ExpectCached("D", "D:3"); } + [Fact] + public void LruBoundedCache_must_handle_explict_set() + { + var cache = new TestCache(4, 4); + + cache.ExpectComputed("A", "A:0"); + cache.TrySet("A", "A:1").Should().Be(true); + cache.Get("A").Should().Be("A:1"); + + cache.TrySet("B", "B:X").Should().Be(true); + cache.Get("B").Should().Be("B:X"); + } + [Fact] public void LruBoundedCache_must_evict_oldest_when_full() { @@ -237,6 +282,8 @@ public void LruBoundedCache_must_not_cache_noncacheable_values() cache.ExpectCached("C", "C:6"); cache.ExpectCached("D", "D:7"); cache.ExpectCached("E", "E:8"); + + cache.TrySet("#X", "#X:13").Should().BeFalse(); } [Fact] diff --git a/src/core/Akka.Remote/Serialization/ActorPathCache.cs b/src/core/Akka.Remote/Serialization/ActorPathCache.cs index d2bef0d7594..249ce9a3d9a 100644 --- a/src/core/Akka.Remote/Serialization/ActorPathCache.cs +++ b/src/core/Akka.Remote/Serialization/ActorPathCache.cs @@ -37,28 +37,62 @@ public static ActorPathThreadLocalCache For(ActorSystem system) /// internal sealed class ActorPathCache : LruBoundedCache { - public ActorPathCache(int capacity = 1024, int evictAgeThreshold = 600) : base(capacity, evictAgeThreshold) + public ActorPathCache(int capacity = 1024, int evictAgeThreshold = 600) + : base(capacity, evictAgeThreshold, FastHashComparer.Default) { } - protected override int Hash(string k) - { - return FastHash.OfStringFast(k); - } - protected override ActorPath Compute(string k) { - //todo lookup in address cache + ActorPath actorPath; + + var path = k.AsSpan(); - if (!ActorPath.TryParseAddress(k, out var address, out var absoluteUri)) + if (!ActorPath.TryParseParts(path, out var addressSpan, out var absoluteUri)) return null; - //todo lookup in root in cache - if (!ActorPath.TryParse(new RootActorPath(address), absoluteUri, out var actorPath)) + string rootPath; + if(absoluteUri.Length > 1 || path.Length > addressSpan.Length) + { + //path end with / + rootPath = path.Slice(0, addressSpan.Length + 1).ToString(); + } + else + { + //todo replace with string.create + Span buffer = addressSpan.Length < 1024 + ? stackalloc char[addressSpan.Length + 1] + : new char[addressSpan.Length + 1]; + path.Slice(0, addressSpan.Length).CopyTo(buffer); + buffer[buffer.Length - 1] = '/'; + rootPath = buffer.ToString(); + } + + //try lookup root in cache + if (!TryGet(rootPath, out actorPath)) + { + if (!Address.TryParse(addressSpan, out var address)) + return null; + + actorPath = new RootActorPath(address); + TrySet(rootPath, actorPath); + } + + if (!ActorPath.TryParse(actorPath, absoluteUri, out actorPath)) return null; return actorPath; + + + } + + private static ActorPath ComputeRootPath(string path) + { + if (!Address.TryParse(path.AsSpan(), out var address)) + return null; + + return new RootActorPath(address); } protected override bool IsCacheable(ActorPath v) diff --git a/src/core/Akka.Remote/Serialization/ActorRefResolveCache.cs b/src/core/Akka.Remote/Serialization/ActorRefResolveCache.cs index e0bebe1de28..9e822b3a1fd 100644 --- a/src/core/Akka.Remote/Serialization/ActorRefResolveCache.cs +++ b/src/core/Akka.Remote/Serialization/ActorRefResolveCache.cs @@ -49,7 +49,8 @@ internal sealed class ActorRefResolveCache : LruBoundedCache { private readonly IRemoteActorRefProvider _provider; - public ActorRefResolveCache(IRemoteActorRefProvider provider, int capacity = 1024, int evictAgeThreshold = 600) : base(capacity, evictAgeThreshold) + public ActorRefResolveCache(IRemoteActorRefProvider provider, int capacity = 1024, int evictAgeThreshold = 600) + : base(capacity, evictAgeThreshold, FastHashComparer.Default) { _provider = provider; } @@ -59,11 +60,6 @@ protected override IActorRef Compute(string k) return _provider.InternalResolveActorRef(k); } - protected override int Hash(string k) - { - return FastHash.OfStringFast(k); - } - protected override bool IsCacheable(IActorRef v) { // don't cache any FutureActorRefs, et al diff --git a/src/core/Akka.Remote/Serialization/AddressCache.cs b/src/core/Akka.Remote/Serialization/AddressCache.cs index cfacfe58b32..a80e4ce49eb 100644 --- a/src/core/Akka.Remote/Serialization/AddressCache.cs +++ b/src/core/Akka.Remote/Serialization/AddressCache.cs @@ -41,15 +41,11 @@ public static AddressThreadLocalCache For(ActorSystem system) /// internal sealed class AddressCache : LruBoundedCache { - public AddressCache(int capacity = 1024, int evictAgeThreshold = 600) : base(capacity, evictAgeThreshold) + public AddressCache(int capacity = 1024, int evictAgeThreshold = 600) + : base(capacity, evictAgeThreshold, FastHashComparer.Default) { } - protected override int Hash(string k) - { - return FastHash.OfStringFast(k); - } - protected override Address Compute(string k) { Address addr; diff --git a/src/core/Akka.Remote/Serialization/LruBoundedCache.cs b/src/core/Akka.Remote/Serialization/LruBoundedCache.cs index 20819f85937..74177bb2672 100644 --- a/src/core/Akka.Remote/Serialization/LruBoundedCache.cs +++ b/src/core/Akka.Remote/Serialization/LruBoundedCache.cs @@ -6,6 +6,7 @@ //----------------------------------------------------------------------- using System; +using System.Collections.Generic; using System.Linq; namespace Akka.Remote.Serialization @@ -25,14 +26,24 @@ internal static class FastHash /// A 32-bit pseudo-random hash value. public static int OfString(string s) { - var chars = s.AsSpan(); + return OfString(s.AsSpan()); + } + + /// + /// Allocatey, but safe implementation of FastHash + /// + /// The input string. + /// A 32-bit pseudo-random hash value. + public static int OfString(ReadOnlySpan s) + { + var len = s.Length; var s0 = 391408L; // seed value 1, DON'T CHANGE var s1 = 601258L; // seed value 2, DON'T CHANGE unchecked { - for(var i = 0; i < chars.Length;i++) + for (var i = 0; i < len; i++) { - var x = s0 ^ chars[i]; // Mix character into PRNG state + var x = s0 ^ s[i]; // Mix character into PRNG state var y = s1; // Xorshift128+ round @@ -82,6 +93,36 @@ public static int OfStringFast(string s) } } } + + + } + + /// + /// INTERNAL API + /// + internal sealed class FastHashComparer : IEqualityComparer + { + public readonly static FastHashComparer Default = new FastHashComparer(); + + public bool Equals(string x, string y) + { + return StringComparer.Ordinal.Equals(x, y); + } + + public bool Equals(ReadOnlySpan x, ReadOnlySpan y) + { + return x.SequenceEqual(y); + } + + public int GetHashCode(string s) + { + return FastHash.OfStringFast(s); + } + + public int GetHashCode(ReadOnlySpan s) + { + return FastHash.OfString(s); + } } /// @@ -103,6 +144,8 @@ public CacheStatistics(int entries, int maxProbeDistance, double averageProbeDis public double AverageProbeDistance { get; } } + + /// /// INTERNAL API /// @@ -117,7 +160,7 @@ public CacheStatistics(int entries, int maxProbeDistance, double averageProbeDis /// The type of value used in the cache. internal abstract class LruBoundedCache where TValue : class { - protected LruBoundedCache(int capacity, int evictAgeThreshold) + protected LruBoundedCache(int capacity, int evictAgeThreshold, IEqualityComparer keyComparer) { if (capacity <= 0) throw new ArgumentOutOfRangeException(nameof(capacity), "Capacity must be larger than zero."); @@ -128,6 +171,7 @@ protected LruBoundedCache(int capacity, int evictAgeThreshold) Capacity = capacity; EvictAgeThreshold = evictAgeThreshold; + _keyComparer = keyComparer; _mask = Capacity - 1; _keys = new TKey[Capacity]; _values = new TValue[Capacity]; @@ -145,6 +189,7 @@ protected LruBoundedCache(int capacity, int evictAgeThreshold) // Practically guarantee an overflow private int _epoch = int.MaxValue - 1; + private readonly IEqualityComparer _keyComparer; private readonly TKey[] _keys; private readonly TValue[] _values; private readonly int[] _hashes; @@ -175,7 +220,7 @@ public CacheStatistics Stats public TValue Get(TKey k) { - var h = Hash(k); + var h = _keyComparer.GetHashCode(k); var position = h & _mask; var probeDistance = 0; @@ -187,12 +232,37 @@ public TValue Get(TKey k) return null; if (probeDistance > otherProbeDistance) return null; - if (_hashes[position] == h && k.Equals(_keys[position])) + if (_hashes[position] == h && _keyComparer.Equals(k, _keys[position])) { return _values[position]; } position = (position + 1) & _mask; - probeDistance = probeDistance + 1; + probeDistance++; + } + } + + public bool TryGet(TKey k, out TValue value) + { + var h = _keyComparer.GetHashCode(k); + + var position = h & _mask; + var probeDistance = 0; + + while (true) + { + var otherProbeDistance = ProbeDistanceOf(position); + if (_values[position] == null || probeDistance > otherProbeDistance) + { + value = default; + return false; + } + if (_hashes[position] == h && _keyComparer.Equals(k, _keys[position])) + { + value = _values[position]; + return true; + } + position = (position + 1) & _mask; + probeDistance++; } } @@ -204,7 +274,7 @@ public TValue GetOrCompute(TKey k) public bool TryGetOrCompute(TKey k, out TValue value) { - var h = Hash(k); + var h = _keyComparer.GetHashCode(k); unchecked { _epoch += 1; } var position = h & _mask; @@ -232,22 +302,69 @@ public bool TryGetOrCompute(TKey k, out TValue value) if (probeDistance > otherProbeDistance) { value = Compute(k); - if (IsCacheable(value)) + if (IsCacheable(value)) Move(position, k, h, value, _epoch, probeDistance); return false; } - else if (_hashes[position] == h && k.Equals(_keys[position])) + else if (_hashes[position] == h && _keyComparer.Equals(k, _keys[position])) { // Update usage _epochs[position] = _epoch; value = _values[position]; - return false; + return true; + } + else + { + // This is not our slot yet + position = (position + 1) & _mask; + probeDistance++; + } + } + } + } + + public bool TrySet(TKey key, TValue value) + { + if (!IsCacheable(value)) return false; + + var h = _keyComparer.GetHashCode(key); + unchecked { _epoch += 1; } + + var position = h & _mask; + var probeDistance = 0; + + while (true) + { + if (_values[position] == null) + { + _keys[position] = key; + _values[position] = value; + _hashes[position] = h; + _epochs[position] = _epoch; + return true; + } + else + { + var otherProbeDistance = ProbeDistanceOf(position); + // If probe distance of the element we try to get is larger than the current slot's, then the element cannot be in + // the table since because of the Robin-Hood property we would have swapped it with the current element. + if (probeDistance > otherProbeDistance) + { + Move(position, key, h, value, _epoch, probeDistance); + return true; + } + else if (_hashes[position] == h && _keyComparer.Equals(key, _keys[position])) + { + // Update usage + _epochs[position] = _epoch; + _values[position] = value; + return true; } else { // This is not our slot yet position = (position + 1) & _mask; - probeDistance = probeDistance + 1; + probeDistance++; } } } @@ -342,9 +459,6 @@ protected int ProbeDistanceOf(int idealSlot, int actualSlot) return ((actualSlot - idealSlot) + Capacity) & _mask; } - - protected abstract int Hash(TKey k); - protected abstract TValue Compute(TKey k); protected abstract bool IsCacheable(TValue v); diff --git a/src/core/Akka/Actor/ActorPath.cs b/src/core/Akka/Actor/ActorPath.cs index aa7df64ca8c..bc66c2ed3e1 100644 --- a/src/core/Akka/Actor/ActorPath.cs +++ b/src/core/Akka/Actor/ActorPath.cs @@ -369,7 +369,7 @@ public ActorPath WithUid(long uid) /// /// Returns a parent of depth - /// 0: Root, 1: Guardian, -1: Parent, -2: GrandParent + /// 0: Root, 1: Guardian, ..., -1: Parent, -2: GrandParent /// /// The parent depth, negative depth for reverse lookup public ActorPath ParentOf(int depth) @@ -488,116 +488,57 @@ public static bool TryParseAddress(string path, out Address address) /// true if the could be parsed, false otherwise. public static bool TryParseAddress(string path, out Address address, out ReadOnlySpan absoluteUri) { - address = null; + address = default; - var spanified = path.AsSpan(); - absoluteUri = spanified; - - var firstColonPos = spanified.IndexOf(':'); - - if (firstColonPos == -1) // not an absolute Uri - return false; - - var fullScheme = SpanHacks.ToLowerInvariant(spanified.Slice(0, firstColonPos)); - if (!fullScheme.StartsWith("akka")) + if (!TryParseParts(path.AsSpan(), out var addressSpan, out absoluteUri)) return false; - spanified = spanified.Slice(firstColonPos + 1); - if (spanified.Length < 2 || !(spanified[0] == '/' && spanified[1] == '/')) + if (!Address.TryParse(addressSpan, out address)) return false; - spanified = spanified.Slice(2); // move past the double // - var firstAtPos = spanified.IndexOf('@'); - string sysName; - - if (firstAtPos == -1) - { - // dealing with an absolute local Uri - var nextSlash = spanified.IndexOf('/'); - - if (nextSlash == -1) - { - sysName = spanified.ToString(); - absoluteUri = "/".AsSpan(); // RELY ON THE JIT - } - else - { - sysName = spanified.Slice(0, nextSlash).ToString(); - absoluteUri = spanified.Slice(nextSlash); - } - - address = new Address(fullScheme, sysName); - return true; - } + return true; + } - // dealing with a remote Uri - sysName = spanified.Slice(0, firstAtPos).ToString(); - spanified = spanified.Slice(firstAtPos + 1); - - /* - * Need to check for: - * - IPV4 / hostnames - * - IPV6 (must be surrounded by '[]') according to spec. - */ - string host; - - // check for IPV6 first - var openBracket = spanified.IndexOf('['); - var closeBracket = spanified.IndexOf(']'); - if (openBracket > -1 && closeBracket > openBracket) + /// + /// Attempts to parse an from a stringified . + /// + /// The string representation of the . + /// A containing the address part. + /// A containing the path following the address. + /// true if the path parts could be parsed, false otherwise. + public static bool TryParseParts(ReadOnlySpan path, out ReadOnlySpan address, out ReadOnlySpan absoluteUri) + { + var firstAtPos = path.IndexOf(':'); + if (firstAtPos < 4 || 255 < firstAtPos) { - // found an IPV6 address - host = spanified.Slice(openBracket, closeBracket - openBracket + 1).ToString(); - spanified = spanified.Slice(closeBracket + 1); // advance past the address - - // need to check for trailing colon - var secondColonPos = spanified.IndexOf(':'); - if (secondColonPos == -1) - return false; - - spanified = spanified.Slice(secondColonPos + 1); + //missing or invalid scheme + address = default; + absoluteUri = path; + return false; } - else + + var doubleSlash = path.Slice(firstAtPos + 1); + if (doubleSlash.Length < 2 || !(doubleSlash[0] == '/' && doubleSlash[1] == '/')) { - var secondColonPos = spanified.IndexOf(':'); - if (secondColonPos == -1) - return false; - - host = spanified.Slice(0, secondColonPos).ToString(); - - // move past the host - spanified = spanified.Slice(secondColonPos + 1); + //missing double slash + address = default; + absoluteUri = path; + return false; } - var actorPathSlash = spanified.IndexOf('/'); - ReadOnlySpan strPort; - if (actorPathSlash == -1) + var nextSlash = path.Slice(firstAtPos + 3).IndexOf('/'); + if (nextSlash == -1) { - strPort = spanified; + address = path; + absoluteUri = "/".AsSpan(); // RELY ON THE JIT } else { - strPort = spanified.Slice(0, actorPathSlash); + address = path.Slice(0, firstAtPos + 3 + nextSlash); + absoluteUri = path.Slice(address.Length); } - - if (SpanHacks.TryParse(strPort, out var port)) - { - address = new Address(fullScheme, sysName, host, port); - - // need to compute the absolute path after the Address - if (actorPathSlash == -1) - { - absoluteUri = "/".AsSpan(); - } - else - { - absoluteUri = spanified.Slice(actorPathSlash); - } - - return true; - } - - return false; + + return true; } diff --git a/src/core/Akka/Actor/Address.cs b/src/core/Akka/Actor/Address.cs index 8b36bd5f38e..6ac1020371c 100644 --- a/src/core/Akka/Actor/Address.cs +++ b/src/core/Akka/Actor/Address.cs @@ -246,7 +246,7 @@ public Address WithPort(int? port = null) /// true if both addresses are equal; otherwise false public static bool operator ==(Address left, Address right) { - return left?.Equals(right) ?? ReferenceEquals(right, null); + return left?.Equals(right) ?? right is null; } /// @@ -299,6 +299,99 @@ public static Address Parse(string address) } } + /// + /// Parses a new from a given string + /// + /// The span of address to parse + /// If true, the parsed . Otherwise null. + /// true if the could be parsed, false otherwise. + public static bool TryParse(ReadOnlySpan span, out Address address) + { + address = default; + + var firstColonPos = span.IndexOf(':'); + + if (firstColonPos == -1) // not an absolute Uri + return false; + + if (firstColonPos < 4 || 255 < firstColonPos) + { + //invalid scheme length + return false; + } + + Span fullScheme = stackalloc char[firstColonPos]; + span.Slice(0, firstColonPos).ToLowerInvariant(fullScheme); + if (!fullScheme.StartsWith("akka".AsSpan())) + { + //invalid scheme + return false; + } + + span = span.Slice(firstColonPos + 1); + if (span.Length < 2 || !(span[0] == '/' && span[1] == '/')) + return false; + + span = span.Slice(2); // move past the double // + var firstAtPos = span.IndexOf('@'); + string sysName; + + if (firstAtPos == -1) + { + // dealing with an absolute local Uri + sysName = span.ToString(); + address = new Address(fullScheme.ToString(), sysName); + return true; + } + + // dealing with a remote Uri + sysName = span.Slice(0, firstAtPos).ToString(); + span = span.Slice(firstAtPos + 1); + + /* + * Need to check for: + * - IPV4 / hostnames + * - IPV6 (must be surrounded by '[]') according to spec. + */ + string host; + + // check for IPV6 first + var openBracket = span.IndexOf('['); + var closeBracket = span.IndexOf(']'); + if (openBracket > -1 && closeBracket > openBracket) + { + // found an IPV6 address + host = span.Slice(openBracket, closeBracket - openBracket + 1).ToString(); + span = span.Slice(closeBracket + 1); // advance past the address + + // need to check for trailing colon + var secondColonPos = span.IndexOf(':'); + if (secondColonPos == -1) + return false; + + span = span.Slice(secondColonPos + 1); + } + else + { + var secondColonPos = span.IndexOf(':'); + if (secondColonPos == -1) + return false; + + host = span.Slice(0, secondColonPos).ToString(); + + // move past the host + span = span.Slice(secondColonPos + 1); + } + + if (SpanHacks.TryParse(span, out var port) && port >= 0) + { + address = new Address(fullScheme.ToString(), sysName, host, port); + return true; + } + + return false; + } + /// /// This class represents a surrogate of an . /// Its main use is to help during the serialization process. From b29e2426db90349770591ccd1e640e2a2cc61337 Mon Sep 17 00:00:00 2001 From: zetanova Date: Wed, 15 Sep 2021 16:35:43 +0200 Subject: [PATCH 24/33] update api --- src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt b/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt index a0b916f0397..b0d83289f84 100644 --- a/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt +++ b/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt @@ -205,6 +205,7 @@ namespace Akka.Actor public static bool TryParse(Akka.Actor.ActorPath basePath, System.ReadOnlySpan absoluteUri, out Akka.Actor.ActorPath actorPath) { } public static bool TryParseAddress(string path, out Akka.Actor.Address address) { } public static bool TryParseAddress(string path, out Akka.Actor.Address address, out System.ReadOnlySpan absoluteUri) { } + public static bool TryParseParts(System.ReadOnlySpan path, out System.ReadOnlySpan address, out System.ReadOnlySpan absoluteUri) { } public Akka.Actor.ActorPath WithUid(long uid) { } public static Akka.Actor.ActorPath /(Akka.Actor.ActorPath path, string name) { } public static Akka.Actor.ActorPath /(Akka.Actor.ActorPath path, System.Collections.Generic.IEnumerable name) { } @@ -413,6 +414,7 @@ namespace Akka.Actor public static Akka.Actor.Address Parse(string address) { } public override string ToString() { } public Akka.Util.ISurrogate ToSurrogate(Akka.Actor.ActorSystem system) { } + public static bool TryParse(System.ReadOnlySpan span, out Akka.Actor.Address address) { } public Akka.Actor.Address WithHost(string host = null) { } public Akka.Actor.Address WithPort(System.Nullable port = null) { } public Akka.Actor.Address WithProtocol(string protocol) { } From 9d9a2d7685e643e7765ee7e362c8488107065228 Mon Sep 17 00:00:00 2001 From: zetanova Date: Wed, 15 Sep 2021 16:42:11 +0200 Subject: [PATCH 25/33] remove obsolete code --- src/core/Akka.Remote/Serialization/ActorPathCache.cs | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/core/Akka.Remote/Serialization/ActorPathCache.cs b/src/core/Akka.Remote/Serialization/ActorPathCache.cs index 249ce9a3d9a..482d349eab4 100644 --- a/src/core/Akka.Remote/Serialization/ActorPathCache.cs +++ b/src/core/Akka.Remote/Serialization/ActorPathCache.cs @@ -82,17 +82,7 @@ protected override ActorPath Compute(string k) if (!ActorPath.TryParse(actorPath, absoluteUri, out actorPath)) return null; - return actorPath; - - - } - - private static ActorPath ComputeRootPath(string path) - { - if (!Address.TryParse(path.AsSpan(), out var address)) - return null; - - return new RootActorPath(address); + return actorPath; } protected override bool IsCacheable(ActorPath v) From 29eaff22c53dc00dddafad4dade240808d111751 Mon Sep 17 00:00:00 2001 From: zetanova Date: Wed, 15 Sep 2021 16:49:27 +0200 Subject: [PATCH 26/33] cleanup code --- src/core/Akka.Remote/Serialization/LruBoundedCache.cs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/core/Akka.Remote/Serialization/LruBoundedCache.cs b/src/core/Akka.Remote/Serialization/LruBoundedCache.cs index 74177bb2672..2e96ecfecea 100644 --- a/src/core/Akka.Remote/Serialization/LruBoundedCache.cs +++ b/src/core/Akka.Remote/Serialization/LruBoundedCache.cs @@ -109,20 +109,10 @@ public bool Equals(string x, string y) return StringComparer.Ordinal.Equals(x, y); } - public bool Equals(ReadOnlySpan x, ReadOnlySpan y) - { - return x.SequenceEqual(y); - } - public int GetHashCode(string s) { return FastHash.OfStringFast(s); } - - public int GetHashCode(ReadOnlySpan s) - { - return FastHash.OfString(s); - } } /// From d3a0ae0aae77ab9a52fc7651f49a251912c22e15 Mon Sep 17 00:00:00 2001 From: zetanova Date: Wed, 15 Sep 2021 20:40:57 +0200 Subject: [PATCH 27/33] removed commented cache tests --- src/core/Akka.Remote.Tests/RemotingSpec.cs | 30 +--------------------- 1 file changed, 1 insertion(+), 29 deletions(-) diff --git a/src/core/Akka.Remote.Tests/RemotingSpec.cs b/src/core/Akka.Remote.Tests/RemotingSpec.cs index 876740461ce..5c36790f4fa 100644 --- a/src/core/Akka.Remote.Tests/RemotingSpec.cs +++ b/src/core/Akka.Remote.Tests/RemotingSpec.cs @@ -177,35 +177,7 @@ public async Task Remoting_must_support_Ask() Assert.Equal("pong", msg); Assert.IsType>(actorRef); } - - //[Fact] - //public async Task Remoting_should_not_cache_ref_of_local_ask() - //{ - // var localActorRefResolveCache = ActorRefResolveThreadLocalCache.For(Sys); - // var localActorPathCache = ActorPathThreadLocalCache.For(Sys); - - // var (msg, actorRef) = await _here.Ask<(string, IActorRef)>("ping", DefaultTimeout); - // Assert.Equal("pong", msg); - // Assert.IsType>(actorRef); - - // Assert.Equal(0, localActorRefResolveCache.All.Sum(n => n.Stats.Entries)); - // Assert.Equal(2, localActorPathCache.All.Sum(n => n.Stats.Entries)); - //} - - //[Fact] - //public async Task Remoting_should_not_cache_ref_of_remote_ask() - //{ - // var remoteActorRefResolveCache = ActorRefResolveThreadLocalCache.For(_remoteSystem); - // var remoteActorPathCache = ActorPathThreadLocalCache.For(_remoteSystem); - - // var (msg, actorRef) = await _here.Ask<(string, IActorRef)>("ping", DefaultTimeout); - // Assert.Equal("pong", msg); - // Assert.IsType>(actorRef); - - // Assert.Equal(0, remoteActorRefResolveCache.All.Sum(n => n.Stats.Entries)); - // Assert.Equal(2, remoteActorPathCache.All.Sum(n => n.Stats.Entries)); //should be 1 - //} - + [Fact(Skip = "Racy")] public async Task Ask_does_not_deadlock() { From 217485c1f751d2e8e95a67a4331cf2b7bb414ed6 Mon Sep 17 00:00:00 2001 From: zetanova Date: Wed, 15 Sep 2021 23:49:08 +0200 Subject: [PATCH 28/33] refactor span to string bulder --- src/core/Akka.Tests/Actor/ActorPathSpec.cs | 31 +++++++++++----------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/core/Akka.Tests/Actor/ActorPathSpec.cs b/src/core/Akka.Tests/Actor/ActorPathSpec.cs index f93c3bfebd2..d5d4ac7509e 100644 --- a/src/core/Akka.Tests/Actor/ActorPathSpec.cs +++ b/src/core/Akka.Tests/Actor/ActorPathSpec.cs @@ -7,11 +7,10 @@ using System; using System.Linq; -using System.Net; +using System.Text; using Akka.Actor; using Akka.TestKit; using Xunit; -using Xunit.Extensions; namespace Akka.Tests.Actor { @@ -27,7 +26,7 @@ public void SupportsParsingItsStringRep() private ActorPath ActorPathParse(string path) { ActorPath actorPath; - if(ActorPath.TryParse(path, out actorPath)) + if (ActorPath.TryParse(path, out actorPath)) return actorPath; throw new UriFormatException(); } @@ -44,12 +43,12 @@ public void ActorPath_Parse_HandlesCasing_ForLocal() // as well as "http") for the sake of robustness but should only produce lowercase scheme names // for consistency." rfc3986 Assert.True(actorPath.Address.Protocol.Equals("akka", StringComparison.Ordinal), "protocol should be lowercase"); - + //In Akka, at least the system name is case-sensitive, see http://doc.akka.io/docs/akka/current/additional/faq.html#what-is-the-name-of-a-remote-actor - Assert.True(actorPath.Address.System.Equals("sYstEm", StringComparison.Ordinal), "system"); + Assert.True(actorPath.Address.System.Equals("sYstEm", StringComparison.Ordinal), "system"); var elements = actorPath.Elements.ToList(); - elements.Count.ShouldBe(2,"number of elements in path"); + elements.Count.ShouldBe(2, "number of elements in path"); Assert.True("pAth1".Equals(elements[0], StringComparison.Ordinal), "first path element"); Assert.True("pAth2".Equals(elements[1], StringComparison.Ordinal), "second path element"); Assert.Equal("akka://sYstEm/pAth1/pAth2", actorPath.ToString()); @@ -114,18 +113,20 @@ public void Return_false_upon_malformed_path() [Fact] public void Supports_jumbo_actor_name_length() { - ReadOnlySpan prefix = "akka://sys@host.domain.com:1234/some/ref/".AsSpan(); - Span b = new char[10 * 1024 * 1024]; //10 MB - prefix.CopyTo(b); - b.Slice(prefix.Length).Fill('a'); - var path = b.ToString(); + var prefix = "akka://sys@host.domain.com:1234/some/ref/"; + var nameSize = 10 * 1024 * 1024; //10MB + + var sb = new StringBuilder(nameSize + prefix.Length); + sb.Append(prefix); + sb.Append('a', nameSize); //10MB + var path = sb.ToString(); ActorPath.TryParse(path, out var actorPath).ShouldBe(true); - actorPath.Name.Length.ShouldBe(b.Length - prefix.Length); + actorPath.Name.Length.ShouldBe(nameSize); actorPath.Name.All(n => n == 'a').ShouldBe(true); var result = actorPath.ToStringWithAddress(); - result.AsSpan().SequenceEqual(b).ShouldBe(true); + result.ShouldBe(path); } [Fact] @@ -213,7 +214,7 @@ public void Paths_with_different_addresses_and_same_elements_should_not_be_equal { ActorPath path1 = null; ActorPath path2 = null; - ActorPath.TryParse("akka.tcp://remotesystem@localhost:8080/user",out path1); + ActorPath.TryParse("akka.tcp://remotesystem@localhost:8080/user", out path1); ActorPath.TryParse("akka://remotesystem/user", out path2); Assert.NotEqual(path2, path1); @@ -255,7 +256,7 @@ public void Validate_element_parts(string element, bool matches) public void Validate_that_url_encoded_values_are_valid_element_parts(string element) { var urlEncode = System.Net.WebUtility.UrlEncode(element); - global::System.Diagnostics.Debug.WriteLine("Encoded \"{0}\" to \"{1}\"", element, urlEncode) ; + global::System.Diagnostics.Debug.WriteLine("Encoded \"{0}\" to \"{1}\"", element, urlEncode); ActorPath.IsValidPathElement(urlEncode).ShouldBeTrue(); } } From 409485f2954ca5ed5eed866f9378b509c2ce375b Mon Sep 17 00:00:00 2001 From: zetanova Date: Fri, 17 Sep 2021 05:00:27 +0200 Subject: [PATCH 29/33] use internal fields and ref equals --- .../Serialization/ActorPathCache.cs | 2 +- .../Serialization/ActorRefResolveCache.cs | 2 +- src/core/Akka/Actor/ActorPath.cs | 25 +++++++++++-------- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/core/Akka.Remote/Serialization/ActorPathCache.cs b/src/core/Akka.Remote/Serialization/ActorPathCache.cs index 482d349eab4..6a933f4d75f 100644 --- a/src/core/Akka.Remote/Serialization/ActorPathCache.cs +++ b/src/core/Akka.Remote/Serialization/ActorPathCache.cs @@ -17,7 +17,7 @@ namespace Akka.Remote.Serialization /// internal sealed class ActorPathThreadLocalCache : ExtensionIdProvider, IExtension { - private readonly ThreadLocal _current = new ThreadLocal(() => new ActorPathCache(), false); + private readonly ThreadLocal _current = new ThreadLocal(() => new ActorPathCache()); public ActorPathCache Cache => _current.Value; diff --git a/src/core/Akka.Remote/Serialization/ActorRefResolveCache.cs b/src/core/Akka.Remote/Serialization/ActorRefResolveCache.cs index 9e822b3a1fd..62d4431503d 100644 --- a/src/core/Akka.Remote/Serialization/ActorRefResolveCache.cs +++ b/src/core/Akka.Remote/Serialization/ActorRefResolveCache.cs @@ -24,7 +24,7 @@ public ActorRefResolveThreadLocalCache() { } public ActorRefResolveThreadLocalCache(IRemoteActorRefProvider provider) { _provider = provider; - _current = new ThreadLocal(() => new ActorRefResolveCache(_provider), false); + _current = new ThreadLocal(() => new ActorRefResolveCache(_provider)); } public override ActorRefResolveThreadLocalCache CreateExtension(ExtendedActorSystem system) diff --git a/src/core/Akka/Actor/ActorPath.cs b/src/core/Akka/Actor/ActorPath.cs index bc66c2ed3e1..20aa0adf452 100644 --- a/src/core/Akka/Actor/ActorPath.cs +++ b/src/core/Akka/Actor/ActorPath.cs @@ -171,7 +171,7 @@ protected ActorPath(Address address, string name) protected ActorPath(ActorPath parentPath, string name, long uid) { _parent = parentPath; - _address = parentPath.Address; + _address = parentPath._address; _depth = parentPath._depth + 1; _name = name; _uid = uid; @@ -222,7 +222,7 @@ public IReadOnlyList Elements var p = this; for (var i = 0; i < _depth; i++) { - b[_depth - i - 1] = p.Name; + b[_depth - i - 1] = p._name; p = p._parent; } return b.MoveToImmutable(); @@ -268,6 +268,9 @@ public bool Equals(ActorPath other) if (other is null || _depth != other._depth) return false; + if (ReferenceEquals(this, other)) + return true; + if (!Address.Equals(other.Address)) return false; @@ -378,12 +381,12 @@ public ActorPath ParentOf(int depth) if (depth >= 0) { while (current._depth > depth) - current = current.Parent; + current = current._parent; } else { - for (var i = depth; i < 0 && current.Depth > 0; i++) - current = current.Parent; + for (var i = depth; i < 0 && current._depth > 0; i++) + current = current._parent; } return current; } @@ -516,7 +519,7 @@ public static bool TryParseParts(ReadOnlySpan path, out ReadOnlySpan absoluteUri = path; return false; } - + var doubleSlash = path.Slice(firstAtPos + 1); if (doubleSlash.Length < 2 || !(doubleSlash[0] == '/' && doubleSlash[1] == '/')) { @@ -537,7 +540,7 @@ public static bool TryParseParts(ReadOnlySpan path, out ReadOnlySpan address = path.Slice(0, firstAtPos + 3 + nextSlash); absoluteUri = path.Slice(address.Length); } - + return true; } @@ -564,7 +567,7 @@ private string Join(ReadOnlySpan prefix) while (p._depth > 0) { totalLength += p._name.Length + 1; - p = p.Parent; + p = p._parent; } // Concatenate segments (in reverse order) into buffer with '/' prefixes @@ -580,7 +583,7 @@ private string Join(ReadOnlySpan prefix) offset -= name.Length + 1; buffer[offset] = '/'; name.CopyTo(buffer.Slice(offset + 1, name.Length)); - p = p.Parent; + p = p._parent; } return buffer.ToString(); //todo use string.Create() when available } @@ -628,8 +631,8 @@ public override int GetHashCode() { var hash = 17; hash = (hash * 23) ^ Address.GetHashCode(); - for (var p = this; !(p is null); p = p.Parent) - hash = (hash * 23) ^ p.Name.GetHashCode(); + for (var p = this; !(p is null); p = p._parent) + hash = (hash * 23) ^ p._name.GetHashCode(); return hash; } } From 9eba407c59963d58e7ee1e212de9f5fc0216c944 Mon Sep 17 00:00:00 2001 From: zetanova Date: Fri, 17 Sep 2021 05:30:32 +0200 Subject: [PATCH 30/33] add rebase path test --- .../CoreAPISpec.ApproveCore.approved.txt | 1 + src/core/Akka.Tests/Actor/ActorPathSpec.cs | 21 +++++++++++++++++++ src/core/Akka/Actor/ActorPath.cs | 13 ++++++++++++ 3 files changed, 35 insertions(+) diff --git a/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt b/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt index 9cd21e0d14d..c8c3861b358 100644 --- a/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt +++ b/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt @@ -202,6 +202,7 @@ namespace Akka.Actor public string ToStringWithoutAddress() { } public Akka.Util.ISurrogate ToSurrogate(Akka.Actor.ActorSystem system) { } public static bool TryParse(string path, out Akka.Actor.ActorPath actorPath) { } + public static bool TryParse(Akka.Actor.ActorPath basePath, string absoluteUri, out Akka.Actor.ActorPath actorPath) { } public static bool TryParse(Akka.Actor.ActorPath basePath, System.ReadOnlySpan absoluteUri, out Akka.Actor.ActorPath actorPath) { } public static bool TryParseAddress(string path, out Akka.Actor.Address address) { } public static bool TryParseAddress(string path, out Akka.Actor.Address address, out System.ReadOnlySpan absoluteUri) { } diff --git a/src/core/Akka.Tests/Actor/ActorPathSpec.cs b/src/core/Akka.Tests/Actor/ActorPathSpec.cs index d5d4ac7509e..b22f2eedc12 100644 --- a/src/core/Akka.Tests/Actor/ActorPathSpec.cs +++ b/src/core/Akka.Tests/Actor/ActorPathSpec.cs @@ -99,6 +99,27 @@ public void Supports_parsing_remote_FQDN_paths() parsed.ToString().ShouldBe(remote); } + [Fact] + public void Supports_rebase_a_path() + { + var path = "akka://sys@host:1234/"; + ActorPath.TryParse(path, out var root).ShouldBe(true); + root.ToString().ShouldBe(path); + + ActorPath.TryParse(root, "/", out var newPath).ShouldBe(true); + newPath.ShouldBe(root); + + var uri1 = "/abc/def"; + ActorPath.TryParse(root, uri1, out newPath).ShouldBe(true); + newPath.ToStringWithAddress().ShouldBe($"{path}{uri1.Substring(1)}"); + newPath.ParentOf(-2).ShouldBe(root); + + var uri2 = "/def"; + ActorPath.TryParse(newPath, uri2, out newPath).ShouldBe(true); + newPath.ToStringWithAddress().ShouldBe($"{path}{uri1.Substring(1)}{uri2}"); + newPath.ParentOf(-3).ShouldBe(root); + } + [Fact] public void Return_false_upon_malformed_path() { diff --git a/src/core/Akka/Actor/ActorPath.cs b/src/core/Akka/Actor/ActorPath.cs index 20aa0adf452..8ddc08ec71f 100644 --- a/src/core/Akka/Actor/ActorPath.cs +++ b/src/core/Akka/Actor/ActorPath.cs @@ -424,6 +424,19 @@ public static bool TryParse(string path, out ActorPath actorPath) return TryParse(new RootActorPath(address), absoluteUri, out actorPath); } + /// + /// Tries to parse the uri, which should be a uri not containing protocol. + /// For example "/user/my-actor" + /// + /// the base path, normaly a root path + /// TBD + /// TBD + /// TBD + public static bool TryParse(ActorPath basePath, string absoluteUri, out ActorPath actorPath) + { + return TryParse(basePath, absoluteUri.AsSpan(), out actorPath); + } + /// /// Tries to parse the uri, which should be a uri not containing protocol. /// For example "/user/my-actor" From cc30aa1813640dabfd1dd0936fde0fb7fba949db Mon Sep 17 00:00:00 2001 From: zetanova Date: Sat, 18 Sep 2021 06:03:07 +0200 Subject: [PATCH 31/33] fix possible NRE --- src/core/Akka.Remote/RemoteActorRefProvider.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/Akka.Remote/RemoteActorRefProvider.cs b/src/core/Akka.Remote/RemoteActorRefProvider.cs index f3165c98791..de8c6d678fe 100644 --- a/src/core/Akka.Remote/RemoteActorRefProvider.cs +++ b/src/core/Akka.Remote/RemoteActorRefProvider.cs @@ -436,7 +436,7 @@ public Deploy LookUpRemotes(IEnumerable p) [MethodImpl(MethodImplOptions.AggressiveInlining)] public bool HasAddress(Address address) { - return address.Equals(RootPath.Address) || Transport.Addresses.Contains(address); + return RootPath.Address.Equals(address) || Transport.Addresses.Contains(address); } /// @@ -485,7 +485,7 @@ public IInternalActorRef ResolveActorRefWithLocalAddress(string path, Address lo ActorPath.TryParse(path, out actorPath); } - if (!HasAddress(actorPath.Address)) + if (!HasAddress(actorPath?.Address)) return CreateRemoteRef(actorPath, localAddress); //the actor's local address was already included in the ActorPath From 73d26fff0ead65414f8715c8e636b593b53ce6d3 Mon Sep 17 00:00:00 2001 From: zetanova Date: Sat, 18 Sep 2021 06:51:24 +0200 Subject: [PATCH 32/33] extend and test address parsing --- src/core/Akka.Tests/Actor/AddressSpec.cs | 17 +++++++ src/core/Akka/Actor/Address.cs | 60 +++++++++++++++++++++++- 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/src/core/Akka.Tests/Actor/AddressSpec.cs b/src/core/Akka.Tests/Actor/AddressSpec.cs index 359ba719c5a..2da9b13a50c 100644 --- a/src/core/Akka.Tests/Actor/AddressSpec.cs +++ b/src/core/Akka.Tests/Actor/AddressSpec.cs @@ -19,6 +19,23 @@ public void Host_is_lowercased_when_created() var address = new Address("akka", "test", "HOSTNAME"); address.Host.ShouldBe("hostname"); } + + [Theory] + [InlineData("akka://sys@host:1234/abc/def/", true, "akka://sys@host:1234", "/abc/def/")] + [InlineData("akka://sys/abc/def/", true, "akka://sys", "/abc/def/")] + [InlineData("akka://host:1234/abc/def/", true, "akka://host:1234", "/abc/def/")] + [InlineData("akka://sys@host:1234", true, "akka://sys@host:1234", "/")] + [InlineData("akka://sys@host:1234/", true, "akka://sys@host:1234", "/")] + [InlineData("akka://sys@host/abc/def/", false, "", "")] + public void Supports_parse_full_actor_path(string path, bool valid, string expectedAddress, string expectedUri) + { + Address.TryParse(path, out var address, out var absolutUri).ShouldBe(valid); + if(valid) + { + address.ToString().ShouldBe(expectedAddress); + absolutUri.ToString().ShouldBe(expectedUri); + } + } } } diff --git a/src/core/Akka/Actor/Address.cs b/src/core/Akka/Actor/Address.cs index 6ac1020371c..0c5fe5e1ae9 100644 --- a/src/core/Akka/Actor/Address.cs +++ b/src/core/Akka/Actor/Address.cs @@ -300,14 +300,57 @@ public static Address Parse(string address) } /// - /// Parses a new from a given string + /// Parses a new from a given path + /// + /// The span of address to parse + /// If true, the parsed . Otherwise null. + /// true if the could be parsed, false otherwise. + public static bool TryParse(string path, out Address address) + { + return TryParse(path.AsSpan(), out address); + } + + /// + /// Parses a new from a given path + /// + /// The span of address to parse + /// If true, the parsed . Otherwise null. + /// If true, the absolut uri of the path. Otherwise default. + /// true if the could be parsed, false otherwise. + public static bool TryParse(string path, out Address address, out string absolutUri) + { + if (TryParse(path.AsSpan(), out address, out var uri)) + { + absolutUri = uri.ToString(); + return true; + } + + absolutUri = default; + return false; + } + + /// + /// Parses a new from a given path /// /// The span of address to parse /// If true, the parsed . Otherwise null. /// true if the could be parsed, false otherwise. public static bool TryParse(ReadOnlySpan span, out Address address) + { + return TryParse(span, out address, out _); + } + + /// + /// Parses a new from a given path + /// + /// The span of address to parse + /// If true, the parsed . Otherwise null. + /// If true, the absolut uri of the path. Otherwise default. + /// true if the could be parsed, false otherwise. + public static bool TryParse(ReadOnlySpan span, out Address address, out ReadOnlySpan absolutUri) { address = default; + absolutUri = default; var firstColonPos = span.IndexOf(':'); @@ -333,6 +376,19 @@ public static bool TryParse(ReadOnlySpan span, out Address address) return false; span = span.Slice(2); // move past the double // + + // cut the absolute Uri off + var uriStart = span.IndexOf('/'); + if (uriStart > -1) + { + absolutUri = span.Slice(uriStart); + span = span.Slice(0, uriStart); + } + else + { + absolutUri = "/".AsSpan(); + } + var firstAtPos = span.IndexOf('@'); string sysName; @@ -383,6 +439,8 @@ public static bool TryParse(ReadOnlySpan span, out Address address) span = span.Slice(secondColonPos + 1); } + + if (SpanHacks.TryParse(span, out var port) && port >= 0) { address = new Address(fullScheme.ToString(), sysName, host, port); From c13ad50ffdf643b85c09c60d00457b34a810753f Mon Sep 17 00:00:00 2001 From: zetanova Date: Sat, 18 Sep 2021 07:12:46 +0200 Subject: [PATCH 33/33] update api --- src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt b/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt index c8c3861b358..615b382d75c 100644 --- a/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt +++ b/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt @@ -415,7 +415,10 @@ namespace Akka.Actor public static Akka.Actor.Address Parse(string address) { } public override string ToString() { } public Akka.Util.ISurrogate ToSurrogate(Akka.Actor.ActorSystem system) { } + public static bool TryParse(string path, out Akka.Actor.Address address) { } + public static bool TryParse(string path, out Akka.Actor.Address address, out string absolutUri) { } public static bool TryParse(System.ReadOnlySpan span, out Akka.Actor.Address address) { } + public static bool TryParse(System.ReadOnlySpan span, out Akka.Actor.Address address, out System.ReadOnlySpan absolutUri) { } public Akka.Actor.Address WithHost(string host = null) { } public Akka.Actor.Address WithPort(System.Nullable port = null) { } public Akka.Actor.Address WithProtocol(string protocol) { }