Skip to content

Commit

Permalink
Reduce FSM<TState, TData> allocations (#6145)
Browse files Browse the repository at this point in the history 
* close #2560 - added performance benchmarks for FSM

* Improved FSM memory consumption

* Made `Event` a `readonly struct`
* Eliminated unnecessary `List<object>` allocations
* Cleaned up XML-DOC comments

* don't return new `State<TState, TData>` during `Stay()`

* API approvals

* fixed `State<TS.,TD>` errors
  • Loading branch information
@Aaronontheweb
Aaronontheweb authored Oct 7, 2022
1 parent a2b27a7 commit 234bb8f
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 33 deletions.
6 changes: 3 additions & 3 deletions src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.verified.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,7 @@ namespace Akka.Actor
public override int GetHashCode() { }
public override string ToString() { }
}
public sealed class Event<TD> : Akka.Actor.INoSerializationVerificationNeeded
public struct Event<TD> : Akka.Actor.INoSerializationVerificationNeeded
{
public Event(object fsmEvent, TD stateData) { }
public object FsmEvent { get; }
Expand Down Expand Up @@ -773,10 +773,10 @@ namespace Akka.Actor
{
public static Akka.Actor.FSMBase.StateTimeout Instance { get; }
}
public class State<TS, TD> : System.IEquatable<Akka.Actor.FSMBase.State<TS, TD>>
public sealed class State<TS, TD> : System.IEquatable<Akka.Actor.FSMBase.State<TS, TD>>
{
public State(TS stateName, TD stateData, System.Nullable<System.TimeSpan> timeout = null, Akka.Actor.FSMBase.Reason stopReason = null, System.Collections.Generic.IReadOnlyList<object> replies = null, bool notifies = True) { }
public System.Collections.Generic.IReadOnlyList<object> Replies { get; set; }
public System.Collections.Generic.IReadOnlyList<object> Replies { get; }
public TD StateData { get; }
public TS StateName { get; }
public Akka.Actor.FSMBase.Reason StopReason { get; }
Expand Down
8 changes: 4 additions & 4 deletions src/core/Akka.Tests/Actor/FSMActorSpec.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -495,20 +495,20 @@ public void FSMActor_must_cancel_all_timers_when_terminated()
var fsmRef = new TestFSMRef<StopTimersFSM, string, object>(Sys, Props.Create(
() => new StopTimersFSM(TestActor, timerNames)));

Action<bool> 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());
Expand Down
70 changes: 44 additions & 26 deletions src/core/Akka/Actor/FSM.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<TS> && Equals((CurrentState<TS>)obj);
return obj is CurrentState<TS> state && Equals(state);
}


Expand Down Expand Up @@ -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<TS> && Equals((Transition<TS>)obj);
return obj is Transition<TS> transition && Equals(transition);
}


Expand Down Expand Up @@ -206,7 +206,7 @@ public abstract class Reason { }
/// </summary>
public sealed class Normal : Reason
{
internal Normal() { }
private Normal() { }

/// <summary>
/// Singleton instance of Normal
Expand All @@ -220,7 +220,7 @@ internal Normal() { }
/// </summary>
public sealed class Shutdown : Reason
{
internal Shutdown() { }
private Shutdown() { }

/// <summary>
/// Singleton instance of Shutdown
Expand Down Expand Up @@ -258,7 +258,7 @@ public Failure(object cause)
/// </summary>
public sealed class StateTimeout
{
internal StateTimeout() { }
private StateTimeout() { }

/// <summary>
/// Singleton instance of StateTimeout
Expand Down Expand Up @@ -422,7 +422,7 @@ public override string ToString()
/// </summary>
/// <typeparam name="TS">The name of the state</typeparam>
/// <typeparam name="TD">The data of the state</typeparam>
public class State<TS, TD> : IEquatable<State<TS, TD>>
public sealed class State<TS, TD> : IEquatable<State<TS, TD>>
{
/// <summary>
/// Initializes a new instance of the State
Expand All @@ -435,7 +435,7 @@ public class State<TS, TD> : IEquatable<State<TS, TD>>
/// <param name="notifies">TBD</param>
public State(TS stateName, TD stateData, TimeSpan? timeout = null, Reason stopReason = null, IReadOnlyList<object> replies = null, bool notifies = true)
{
Replies = replies ?? new List<object>();
Replies = replies ?? Array.Empty<object>();
StopReason = stopReason;
Timeout = timeout;
StateData = stateData;
Expand All @@ -444,32 +444,33 @@ public State(TS stateName, TD stateData, TimeSpan? timeout = null, Reason stopRe
}

/// <summary>
/// TBD
/// The name of this state
/// </summary>
public TS StateName { get; }

/// <summary>
/// TBD
/// The data belonging to this sate
/// </summary>
public TD StateData { get; }

/// <summary>
/// TBD
/// Optional. The state timeout.
/// </summary>
public TimeSpan? Timeout { get; }

/// <summary>
/// TBD
/// Optional - the reason why we're stopping.
/// </summary>
public Reason StopReason { get; }

/// <summary>
/// TBD
/// Optional - the set of replies to send to subscribers.
/// </summary>
public IReadOnlyList<object> Replies { get; protected set; }
public IReadOnlyList<object> Replies { get; }

/// <summary>
/// TBD
/// INTERNAL API. Indicates whether or not we're sending transition notifications
/// for this state change.
/// </summary>
internal bool Notifies { get; }

Expand All @@ -482,9 +483,24 @@ public State(TS stateName, TD stateData, TimeSpan? timeout = null, Reason stopRe
/// <returns>TBD</returns>
internal State<TS, TD> Copy(TimeSpan? timeout, Reason stopReason = null, IReadOnlyList<object> replies = null)
{
// otherwise, return a new copy.
return new State<TS, TD>(StateName, StateData, timeout, stopReason ?? StopReason, replies ?? Replies, Notifies);
}

/// <summary>
/// Remove all of the unwanted bits we don't need during a Stay() operation.
/// </summary>
/// <remarks>
/// This is a performance optimization designed to
/// </remarks>
/// <returns>Returns a "clean" copy of a state object the first time it's called. Idempotent after.</returns>
internal State<TS, TD> SpecialCopyForStay()
{
if(Replies.Count > 0 || StopReason != null || Timeout != null || Notifies)
return new State<TS, TD>(StateName, StateData, notifies:false);
return this;
}

/// <summary>
/// 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.
Expand Down Expand Up @@ -539,15 +555,16 @@ internal State<TS, TD> WithStopReason(Reason reason)
/// </summary>
internal State<TS, TD> 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<TS, TD>(StateName, StateData, Timeout, StopReason, Replies, notifies);
}


public override string ToString()
{
return $"{StateName}, {StateData}";
}


public bool Equals(State<TS, TD> other)
{
Expand Down Expand Up @@ -585,26 +602,26 @@ public override int GetHashCode()
/// which allows pattern matching to extract both state and data.
/// </summary>
/// <typeparam name="TD">The state data for this event</typeparam>
public sealed class Event<TD> : INoSerializationVerificationNeeded
public readonly struct Event<TD> : INoSerializationVerificationNeeded
{
/// <summary>
/// Initializes a new instance of the Event
/// </summary>
/// <param name="fsmEvent">TBD</param>
/// <param name="stateData">TBD</param>
/// <param name="fsmEvent">The message received by the FSM.</param>
/// <param name="stateData">The current state data of the FSM.</param>
public Event(object fsmEvent, TD stateData)
{
StateData = stateData;
FsmEvent = fsmEvent;
}

/// <summary>
/// TBD
/// The message received by the FSM.
/// </summary>
public object FsmEvent { get; }

/// <summary>
/// TBD
/// The current state data of the FSM.
/// </summary>
public TD StateData { get; }

Expand Down Expand Up @@ -750,7 +767,8 @@ public State<TState, TData> GoTo(TState nextStateName, TData stateData)
/// <returns>Descriptor for staying in the current state.</returns>
public State<TState, TData> Stay()
{
return GoTo(_currentState.StateName).WithNotification(false);
// don't want to allocate a new state if we don't have to
return _currentState.SpecialCopyForStay();
}

/// <summary>
Expand Down Expand Up @@ -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.");
}
Expand Down Expand Up @@ -1175,7 +1193,7 @@ private void ProcessEvent(Event<TData> fsmEvent, object source)
var stateFunc = _stateFunctions[_currentState.StateName];
var oldState = _currentState;

State<TState, TData> nextState = null;
State<TState, TData> nextState = default;

if (stateFunc != null)
{
Expand All @@ -1195,7 +1213,7 @@ private void ProcessEvent(Event<TData> fsmEvent, object source)
}
}

private string GetSourceString(object source)
private static string GetSourceString(object source)
{
if (source is string s)
return s;
Expand Down Expand Up @@ -1249,7 +1267,7 @@ private void MakeTransition(State<TState, TData> nextState)
_nextState = nextState;
HandleTransition(_currentState.StateName, nextState.StateName);
Listeners.Gossip(new Transition<TState>(Self, _currentState.StateName, nextState.StateName));
_nextState = null;
_nextState = default(State<TState, TData>);
}
_currentState = nextState;

Expand Down

0 comments on commit 234bb8f

Please sign in to comment.