From 6b50d9368d06a4a80fe161399599421473ffe738 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Mon, 19 Apr 2021 16:34:40 -0500 Subject: [PATCH 1/4] reduced iteration count to speed up benchmarks --- src/benchmark/Akka.Benchmarks/Cluster/ReachabilityBenchmarks.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/benchmark/Akka.Benchmarks/Cluster/ReachabilityBenchmarks.cs b/src/benchmark/Akka.Benchmarks/Cluster/ReachabilityBenchmarks.cs index 32869c18869..1aa6a6323b2 100644 --- a/src/benchmark/Akka.Benchmarks/Cluster/ReachabilityBenchmarks.cs +++ b/src/benchmark/Akka.Benchmarks/Cluster/ReachabilityBenchmarks.cs @@ -13,7 +13,7 @@ namespace Akka.Benchmarks.Cluster [Config(typeof(MicroBenchmarkConfig))] public class ReachabilityBenchmarks { - [Params(10, 100, 250)] + [Params(100)] public int NodesSize; [Params(100)] From 9a11cb01fefd42f30a24f4a3b0764a54296c3750 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Mon, 19 Apr 2021 16:56:34 -0500 Subject: [PATCH 2/4] optimize some System.Collections.Immutable invocations to allocate less --- src/core/Akka.Cluster/Reachability.cs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/core/Akka.Cluster/Reachability.cs b/src/core/Akka.Cluster/Reachability.cs index c7da1f16412..65dfef3238f 100644 --- a/src/core/Akka.Cluster/Reachability.cs +++ b/src/core/Akka.Cluster/Reachability.cs @@ -467,10 +467,10 @@ public Cache(ImmutableList records) { if (records.IsEmpty) { - ObserverRowMap = ImmutableDictionary - .Create>(); - AllTerminated = ImmutableHashSet.Create(); - AllUnreachable = ImmutableHashSet.Create(); + ObserverRowMap = ImmutableDictionary> + .Empty; + AllTerminated = ImmutableHashSet.Empty; + AllUnreachable = ImmutableHashSet.Empty; } else { @@ -483,10 +483,7 @@ public Cache(ImmutableList records) ImmutableDictionary m = mapBuilder.TryGetValue(r.Observer, out m) ? m.SetItem(r.Subject, r) //TODO: Other collections take items for Create. Create unnecessary array here - : ImmutableDictionary.CreateRange(new[] - { - new KeyValuePair(r.Subject, r) - }); + : ImmutableDictionary.Empty.Add(r.Subject, r); mapBuilder.AddOrSet(r.Observer, m); @@ -514,12 +511,12 @@ public ImmutableDictionary - /// TBD + /// Contains all nodes that have been observed as Terminated by at least one other node. /// public ImmutableHashSet AllTerminated { get; } /// - /// TBD + /// Contains all nodes that have been observed as Unreachable by at least one other node. /// public ImmutableHashSet AllUnreachable { get; } From 630cf51dda2af87ecde1b73ca54c8796f4f7338a Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Mon, 19 Apr 2021 17:11:41 -0500 Subject: [PATCH 3/4] cleanup dictionary construction --- src/core/Akka.Cluster/Reachability.cs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/core/Akka.Cluster/Reachability.cs b/src/core/Akka.Cluster/Reachability.cs index 65dfef3238f..04c54552caf 100644 --- a/src/core/Akka.Cluster/Reachability.cs +++ b/src/core/Akka.Cluster/Reachability.cs @@ -337,7 +337,7 @@ public bool IsReachable(UniqueAddress observer, UniqueAddress subject) public ImmutableHashSet AllUnreachableFrom(UniqueAddress observer) { var observerRows = ObserverRows(observer); - if (observerRows == null) return ImmutableHashSet.Create(); + if (observerRows == null) return ImmutableHashSet.Empty; return ImmutableHashSet.CreateRange( observerRows.Where(p => p.Value.Status == ReachabilityStatus.Unreachable).Select(p => p.Key)); @@ -351,16 +351,18 @@ public ImmutableHashSet AllUnreachableFrom(UniqueAddress observer public ImmutableList RecordsFrom(UniqueAddress observer) { var rows = ObserverRows(observer); - if (rows == null) return ImmutableList.Create(); + if (rows == null) return ImmutableList.Empty; return rows.Values.ToImmutableList(); } + /// only used for testing /// public override int GetHashCode() { return Versions.GetHashCode(); } + /// only used for testing /// public override bool Equals(object obj) { @@ -480,13 +482,12 @@ public Cache(ImmutableList records) foreach (var r in records) { - ImmutableDictionary m = mapBuilder.TryGetValue(r.Observer, out m) - ? m.SetItem(r.Subject, r) - //TODO: Other collections take items for Create. Create unnecessary array here + ImmutableDictionary m = mapBuilder.TryGetValue(r.Observer, out var mR) + ? mR.SetItem(r.Subject, r) : ImmutableDictionary.Empty.Add(r.Subject, r); - mapBuilder.AddOrSet(r.Observer, m); + mapBuilder[r.Observer] = m; if (r.Status == ReachabilityStatus.Unreachable) unreachableBuilder.Add(r.Subject); else if (r.Status == ReachabilityStatus.Terminated) terminatedBuilder.Add(r.Subject); From 73f4edbd181f53a9617d6f0b1a4eaef541faaeca Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Tue, 20 Apr 2021 10:06:05 -0500 Subject: [PATCH 4/4] fixed multiple enumeration bug in `Reachability` --- .../Cluster/ReachabilityBenchmarks.cs | 5 +++-- src/core/Akka.Cluster/Gossip.cs | 2 +- src/core/Akka.Cluster/Reachability.cs | 12 +++++------- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/benchmark/Akka.Benchmarks/Cluster/ReachabilityBenchmarks.cs b/src/benchmark/Akka.Benchmarks/Cluster/ReachabilityBenchmarks.cs index 1aa6a6323b2..fc8362a2bd8 100644 --- a/src/benchmark/Akka.Benchmarks/Cluster/ReachabilityBenchmarks.cs +++ b/src/benchmark/Akka.Benchmarks/Cluster/ReachabilityBenchmarks.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Collections.Immutable; using System.Linq; using Akka.Util; using Akka.Actor; @@ -51,7 +52,7 @@ private Reachability AddUnreachable(Reachability baseReachability, int count) internal Reachability Reachability1; internal Reachability Reachability2; internal Reachability Reachability3; - internal HashSet Allowed; + internal ImmutableHashSet Allowed; [GlobalSetup] public void Setup() @@ -59,7 +60,7 @@ public void Setup() Reachability1 = CreateReachabilityOfSize(Reachability.Empty, NodesSize); Reachability2 = CreateReachabilityOfSize(Reachability1, NodesSize); Reachability3 = AddUnreachable(Reachability1, NodesSize / 2); - Allowed = Reachability1.Versions.Keys.ToHashSet(); + Allowed = Reachability1.Versions.Keys.ToImmutableHashSet(); } private void CheckThunkFor(Reachability r1, Reachability r2, Action thunk, diff --git a/src/core/Akka.Cluster/Gossip.cs b/src/core/Akka.Cluster/Gossip.cs index 3777459fd95..0a680a4d939 100644 --- a/src/core/Akka.Cluster/Gossip.cs +++ b/src/core/Akka.Cluster/Gossip.cs @@ -282,7 +282,7 @@ public Gossip Merge(Gossip that) var mergedMembers = EmptyMembers.Union(Member.PickHighestPriority(this._members, that._members)); // 3. merge reachability table by picking records with highest version - var mergedReachability = _overview.Reachability.Merge(mergedMembers.Select(m => m.UniqueAddress), + var mergedReachability = _overview.Reachability.Merge(mergedMembers.Select(m => m.UniqueAddress).ToImmutableSortedSet(), that._overview.Reachability); // 4. Nobody can have seen this new gossip yet diff --git a/src/core/Akka.Cluster/Reachability.cs b/src/core/Akka.Cluster/Reachability.cs index 04c54552caf..133f36a159a 100644 --- a/src/core/Akka.Cluster/Reachability.cs +++ b/src/core/Akka.Cluster/Reachability.cs @@ -55,7 +55,6 @@ public enum ReachabilityStatus public static readonly Reachability Empty = new Reachability(ImmutableList.Create(), ImmutableDictionary.Create()); - //TODO: Serialization should ignore private readonly Lazy _cache; /// @@ -80,11 +79,6 @@ public Reachability(ImmutableList records, ImmutableDictionary public ImmutableDictionary Versions { get; } - /* - * def isReachable(observer: UniqueAddress, subject: UniqueAddress): Boolean = - status(observer, subject) == Reachable - */ - /// /// TBD /// @@ -178,7 +172,11 @@ private Reachability Change(UniqueAddress observer, UniqueAddress subject, Reach var newVersions = Versions.SetItem(observer, v); var newRecord = new Record(observer, subject, status, v); var oldObserverRows = ObserverRows(observer); + + // don't record Reachable observation if nothing has been noted so far if (oldObserverRows == null && status == ReachabilityStatus.Reachable) return this; + + // otherwise, create new instance including this first observation if (oldObserverRows == null) return new Reachability(Records.Add(newRecord), newVersions); if (!oldObserverRows.TryGetValue(subject, out var oldRecord)) @@ -206,7 +204,7 @@ private Reachability Change(UniqueAddress observer, UniqueAddress subject, Reach /// TBD /// TBD /// TBD - public Reachability Merge(IEnumerable allowed, Reachability other) + public Reachability Merge(IImmutableSet allowed, Reachability other) { var recordBuilder = ImmutableList.CreateBuilder(); //TODO: Size hint somehow?