RemoteActorRefProvider address paring, caching and resolving improvements

src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt

@@ -177,18 +177,21 @@ 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<string> Elements { get; }
+        public int Depth { get; }
+        public System.Collections.Generic.IReadOnlyList<string> 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<string> 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) { }
@@ -199,13 +202,15 @@ 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<char> absoluteUri, 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 static bool TryParseAddress(string path, out Akka.Actor.Address address, out System.ReadOnlySpan<char> 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<string> 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<Akka.Actor.ActorPath.Surrogate>, System.IEquatable<Akka.Actor.ActorPath>
+        public sealed class Surrogate : Akka.Util.ISurrogate, System.IEquatable<Akka.Actor.ActorPath.Surrogate>, System.IEquatable<Akka.Actor.ActorPath>
         {
             public Surrogate(string path) { }
             public string Path { get; }
@@ -414,7 +419,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 +502,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<string> 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 +1543,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<string> 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

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<FutureActorRef<(string, IActorRef)>>(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<FutureActorRef<(string, IActorRef)>>(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<FutureActorRef<(string, IActorRef)>>(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()
         {

src/core/Akka.Remote/RemoteActorRefProvider.cs

@@ -79,7 +79,7 @@ public interface IRemoteActorRefProvider : IActorRefProvider
         /// <see cref="IActorRefProvider.ResolveActorRef(string)"/> method.
         /// </summary>
         /// <param name="path">The path of the actor we intend to resolve.</param>
-        /// <returns>An <see cref="IActorRef"/> if a match was found. Otherwise nobody.</returns>
+        /// <returns>An <see cref="IActorRef"/> if a match was found. Otherwise deadletters.</returns>
         IActorRef InternalResolveActorRef(string path);
 
         /// <summary>
@@ -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,34 +235,34 @@ public void UnregisterTempActor(ActorPath path)
             _local.UnregisterTempActor(path);
         }
 
-        private volatile IActorRef _remotingTerminator;
-        private volatile IActorRef _remoteWatcher;
+        private IActorRef _remotingTerminator;        
+        private IActorRef _remoteWatcher;
 
-        private volatile ActorRefResolveThreadLocalCache _actorRefResolveThreadLocalCache;
-        private volatile ActorPathThreadLocalCache _actorPathThreadLocalCache;
+        private ActorRefResolveThreadLocalCache _actorRefResolveThreadLocalCache;
+        private ActorPathThreadLocalCache _actorPathThreadLocalCache;
 
         /// <summary>
         /// The remote death watcher.
         /// </summary>
         public IActorRef RemoteWatcher => _remoteWatcher;
-        private volatile IActorRef _remoteDeploymentWatcher;
+        private IActorRef _remoteDeploymentWatcher;
 
         /// <inheritdoc/>
         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))),
                     "remoting-terminator");
 
-            _internals = CreateInternals();
+            _internals = CreateInternals();                              
 
             _remotingTerminator.Tell(RemoteInternals);
 
@@ -433,9 +433,10 @@ public Deploy LookUpRemotes(IEnumerable<string> p)
             return Deploy.None;
         }
 
+        [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);
         }
 
         /// <summary>
@@ -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);
-            }
-        }
-
-
         /// <summary>
         /// INTERNAL API.
         ///
@@ -483,20 +469,31 @@ private bool TryParseCachedPath(string actorPath, out ActorPath path)
         /// <returns>TBD</returns>
         public IInternalActorRef ResolveActorRefWithLocalAddress(string path, Address localAddress)
         {
-            if (TryParseCachedPath(path, out var actorPath))
+            if (path is 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
-                }
+                _log.Debug("resolve of unknown path [{0}] failed", path);
+                return InternalDeadLetters;
+            }
 
-                return CreateRemoteRef(new RootActorPath(actorPath.Address) / actorPath.ElementsWithUid, localAddress);
+            ActorPath actorPath;
+            if (_actorPathThreadLocalCache != null)
+            {
+                actorPath = _actorPathThreadLocalCache.Cache.GetOrCompute(path);
             }
-            _log.Debug("resolve of unknown path [{0}] failed", path);
-            return InternalDeadLetters;
+            else // cache not initialized yet
+            {
+                ActorPath.TryParse(path, out actorPath);
+            }
+
+            if (!HasAddress(actorPath.Address))
+                return CreateRemoteRef(actorPath, 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
         }
 
 
@@ -539,7 +536,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);
         }
@@ -592,19 +590,20 @@ public IActorRef ResolveActorRef(ActorPath actorPath)
         /// <returns>The remote Address, if applicable. If not applicable <c>null</c> may be returned.</returns>
         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;
         }
 
         /// <summary>

src/core/Akka.Remote/RemoteSystemDaemon.cs

@@ -28,7 +28,7 @@ internal interface IDaemonMsg { }
     /// <summary>
     ///  INTERNAL API
     /// </summary>
-    internal class DaemonMsgCreate : IDaemonMsg
+    internal sealed class DaemonMsgCreate : IDaemonMsg
     {
         /// <summary>
         ///     Initializes a new instance of the <see cref="DaemonMsgCreate" /> 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.
     /// </summary>
-    internal class RemoteSystemDaemon : VirtualPathContainer
+    internal sealed class RemoteSystemDaemon : VirtualPathContainer
     {
         private readonly ActorSystemImpl _system;
         private readonly Switch _terminating = new Switch(false);

src/core/Akka.Remote/Remoting.cs

@@ -111,7 +111,7 @@ internal interface IPriorityMessage { }
     /// <summary>
     /// INTERNAL API
     /// </summary>
-    internal class Remoting : RemoteTransport
+    internal sealed class Remoting : RemoteTransport
     {
         private readonly ILoggingAdapter _log;
         private volatile IDictionary<string, HashSet<ProtocolTransportAddressPair>> _transportMapping;

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,7 +17,7 @@ namespace Akka.Remote.Serialization
     /// </summary>
     internal sealed class ActorPathThreadLocalCache : ExtensionIdProvider<ActorPathThreadLocalCache>, IExtension
     {
-        private readonly ThreadLocal<ActorPathCache> _current = new ThreadLocal<ActorPathCache>(() => new ActorPathCache());
+        private readonly ThreadLocal<ActorPathCache> _current = new ThreadLocal<ActorPathCache>(() => new ActorPathCache(), false);
 
         public ActorPathCache Cache => _current.Value;
 
@@ -47,9 +48,17 @@ protected override int Hash(string k)
 
         protected override ActorPath Compute(string k)
         {
-            if (ActorPath.TryParse(k, out var actorPath))
-                return actorPath;
-            return null;
+            //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;
         }
 
         protected override bool IsCacheable(ActorPath v)

src/core/Akka.Remote/Serialization/ActorRefResolveCache.cs

@@ -5,6 +5,7 @@
 // </copyright>
 //-----------------------------------------------------------------------
 
+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<ActorRefResolveCache>(() => new ActorRefResolveCache(_provider));
+            _current = new ThreadLocal<ActorRefResolveCache>(() => new ActorRefResolveCache(_provider), false);
         }
 
         public override ActorRefResolveThreadLocalCache CreateExtension(ExtendedActorSystem system)