From 234bb8fe671eb3ff6ecc2cf3dcca514b6830ffe5 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Fri, 7 Oct 2022 10:59:30 -0500 Subject: [PATCH] Reduce `FSM` allocations (#6145) * close #2560 - added performance benchmarks for FSM * Improved FSM memory consumption * Made `Event` a `readonly struct` * Eliminated unnecessary `List` allocations * Cleaned up XML-DOC comments * don't return new `State` during `Stay()` * API approvals * fixed `State` errors --- .../CoreAPISpec.ApproveCore.verified.txt | 6 +- src/core/Akka.Tests/Actor/FSMActorSpec.cs | 8 +-- src/core/Akka/Actor/FSM.cs | 70 ++++++++++++------- 3 files changed, 51 insertions(+), 33 deletions(-) diff --git a/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.verified.txt b/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.verified.txt index 3e3a0702e48..c04f2b3a683 100644 --- a/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.verified.txt +++ b/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.verified.txt @@ -736,7 +736,7 @@ namespace Akka.Actor public override int GetHashCode() { } public override string ToString() { } } - public sealed class Event : Akka.Actor.INoSerializationVerificationNeeded + public struct Event : Akka.Actor.INoSerializationVerificationNeeded { public Event(object fsmEvent, TD stateData) { } public object FsmEvent { get; } @@ -773,10 +773,10 @@ namespace Akka.Actor { public static Akka.Actor.FSMBase.StateTimeout Instance { get; } } - public class State : System.IEquatable> + public sealed class State : System.IEquatable> { public State(TS stateName, TD stateData, System.Nullable timeout = null, Akka.Actor.FSMBase.Reason stopReason = null, System.Collections.Generic.IReadOnlyList replies = null, bool notifies = True) { } - public System.Collections.Generic.IReadOnlyList Replies { get; set; } + public System.Collections.Generic.IReadOnlyList Replies { get; } public TD StateData { get; } public TS StateName { get; } public Akka.Actor.FSMBase.Reason StopReason { get; } diff --git a/src/core/Akka.Tests/Actor/FSMActorSpec.cs b/src/core/Akka.Tests/Actor/FSMActorSpec.cs index 128a394cb45..1ee04f2c10b 100644 --- a/src/core/Akka.Tests/Actor/FSMActorSpec.cs +++ b/src/core/Akka.Tests/Actor/FSMActorSpec.cs @@ -495,20 +495,20 @@ public void FSMActor_must_cancel_all_timers_when_terminated() var fsmRef = new TestFSMRef(Sys, Props.Create( () => new StopTimersFSM(TestActor, timerNames))); - Action checkTimersActive = active => + void CheckTimersActive(bool active) { foreach (var timer in timerNames) { fsmRef.IsTimerActive(timer).Should().Be(active); fsmRef.IsStateTimerActive().Should().Be(active); } - }; + } - checkTimersActive(false); + CheckTimersActive(false); fsmRef.Tell("start"); ExpectMsg("starting", 1.Seconds()); - checkTimersActive(true); + CheckTimersActive(true); fsmRef.Tell("stop"); ExpectMsg("stopped", 1.Seconds()); diff --git a/src/core/Akka/Actor/FSM.cs b/src/core/Akka/Actor/FSM.cs index 93ba291910c..c1dd1017978 100644 --- a/src/core/Akka/Actor/FSM.cs +++ b/src/core/Akka/Actor/FSM.cs @@ -63,7 +63,7 @@ public override bool Equals(object obj) { if (ReferenceEquals(null, obj)) return false; if (ReferenceEquals(this, obj)) return true; - return obj is CurrentState && Equals((CurrentState)obj); + return obj is CurrentState state && Equals(state); } @@ -129,7 +129,7 @@ public override bool Equals(object obj) { if (ReferenceEquals(null, obj)) return false; if (ReferenceEquals(this, obj)) return true; - return obj is Transition && Equals((Transition)obj); + return obj is Transition transition && Equals(transition); } @@ -206,7 +206,7 @@ public abstract class Reason { } /// public sealed class Normal : Reason { - internal Normal() { } + private Normal() { } /// /// Singleton instance of Normal @@ -220,7 +220,7 @@ internal Normal() { } /// public sealed class Shutdown : Reason { - internal Shutdown() { } + private Shutdown() { } /// /// Singleton instance of Shutdown @@ -258,7 +258,7 @@ public Failure(object cause) /// public sealed class StateTimeout { - internal StateTimeout() { } + private StateTimeout() { } /// /// Singleton instance of StateTimeout @@ -422,7 +422,7 @@ public override string ToString() /// /// The name of the state /// The data of the state - public class State : IEquatable> + public sealed class State : IEquatable> { /// /// Initializes a new instance of the State @@ -435,7 +435,7 @@ public class State : IEquatable> /// TBD public State(TS stateName, TD stateData, TimeSpan? timeout = null, Reason stopReason = null, IReadOnlyList replies = null, bool notifies = true) { - Replies = replies ?? new List(); + Replies = replies ?? Array.Empty(); StopReason = stopReason; Timeout = timeout; StateData = stateData; @@ -444,32 +444,33 @@ public State(TS stateName, TD stateData, TimeSpan? timeout = null, Reason stopRe } /// - /// TBD + /// The name of this state /// public TS StateName { get; } /// - /// TBD + /// The data belonging to this sate /// public TD StateData { get; } /// - /// TBD + /// Optional. The state timeout. /// public TimeSpan? Timeout { get; } /// - /// TBD + /// Optional - the reason why we're stopping. /// public Reason StopReason { get; } /// - /// TBD + /// Optional - the set of replies to send to subscribers. /// - public IReadOnlyList Replies { get; protected set; } + public IReadOnlyList Replies { get; } /// - /// TBD + /// INTERNAL API. Indicates whether or not we're sending transition notifications + /// for this state change. /// internal bool Notifies { get; } @@ -482,9 +483,24 @@ public State(TS stateName, TD stateData, TimeSpan? timeout = null, Reason stopRe /// TBD internal State Copy(TimeSpan? timeout, Reason stopReason = null, IReadOnlyList replies = null) { + // otherwise, return a new copy. return new State(StateName, StateData, timeout, stopReason ?? StopReason, replies ?? Replies, Notifies); } + /// + /// Remove all of the unwanted bits we don't need during a Stay() operation. + /// + /// + /// This is a performance optimization designed to + /// + /// Returns a "clean" copy of a state object the first time it's called. Idempotent after. + internal State SpecialCopyForStay() + { + if(Replies.Count > 0 || StopReason != null || Timeout != null || Notifies) + return new State(StateName, StateData, notifies:false); + return this; + } + /// /// Modify the state transition descriptor to include a state timeout for the /// next state. This timeout overrides any default timeout set for the next state. @@ -539,15 +555,16 @@ internal State WithStopReason(Reason reason) /// internal State WithNotification(bool notifies) { + // don't bother allocating even a stack type if the notifies value is identical. + if (Notifies == notifies) + return this; return new State(StateName, StateData, Timeout, StopReason, Replies, notifies); } - public override string ToString() { return $"{StateName}, {StateData}"; } - public bool Equals(State other) { @@ -585,13 +602,13 @@ public override int GetHashCode() /// which allows pattern matching to extract both state and data. /// /// The state data for this event - public sealed class Event : INoSerializationVerificationNeeded + public readonly struct Event : INoSerializationVerificationNeeded { /// /// Initializes a new instance of the Event /// - /// TBD - /// TBD + /// The message received by the FSM. + /// The current state data of the FSM. public Event(object fsmEvent, TD stateData) { StateData = stateData; @@ -599,12 +616,12 @@ public Event(object fsmEvent, TD stateData) } /// - /// TBD + /// The message received by the FSM. /// public object FsmEvent { get; } /// - /// TBD + /// The current state data of the FSM. /// public TD StateData { get; } @@ -750,7 +767,8 @@ public State GoTo(TState nextStateName, TData stateData) /// Descriptor for staying in the current state. public State Stay() { - return GoTo(_currentState.StateName).WithNotification(false); + // don't want to allocate a new state if we don't have to + return _currentState.SpecialCopyForStay(); } /// @@ -940,7 +958,7 @@ public TState StateName { get { - if (_currentState != null) + if (_currentState != null) return _currentState.StateName; throw new IllegalStateException("You must call StartWith before calling StateName."); } @@ -1175,7 +1193,7 @@ private void ProcessEvent(Event fsmEvent, object source) var stateFunc = _stateFunctions[_currentState.StateName]; var oldState = _currentState; - State nextState = null; + State nextState = default; if (stateFunc != null) { @@ -1195,7 +1213,7 @@ private void ProcessEvent(Event fsmEvent, object source) } } - private string GetSourceString(object source) + private static string GetSourceString(object source) { if (source is string s) return s; @@ -1249,7 +1267,7 @@ private void MakeTransition(State nextState) _nextState = nextState; HandleTransition(_currentState.StateName, nextState.StateName); Listeners.Gossip(new Transition(Self, _currentState.StateName, nextState.StateName)); - _nextState = null; + _nextState = default(State); } _currentState = nextState;