Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reduce allocations inside PhiAccrualFailureDetector #4913

Merged
merged 1 commit into from
Apr 7, 2021
Merged

reduce allocations inside PhiAccrualFailureDetector #4913

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 29 additions & 45 deletions src/core/Akka.Remote/PhiAccrualFailureDetector.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using Akka.Actor;
using Akka.Configuration;
Expand Down Expand Up @@ -35,12 +36,12 @@ namespace Akka.Remote
/// </summary>
public class PhiAccrualFailureDetector : FailureDetector
{
private double _threshold;
private int _maxSampleSize;
private readonly double _threshold;
private readonly int _maxSampleSize;
private TimeSpan _minStdDeviation;
private TimeSpan _acceptableHeartbeatPause;
private TimeSpan _firstHeartbeatEstimate;
private Clock _clock;
private readonly Clock _clock;

/// <summary>
/// Procedural constructor for PhiAccrualDetector
Expand Down Expand Up @@ -182,7 +183,7 @@ public override void HeartBeat()
{
var timestamp = _clock();
var oldState = state;
HeartbeatHistory newHistory = null;
HeartbeatHistory newHistory;

if (!oldState.TimeStamp.HasValue)
{
Expand Down Expand Up @@ -275,14 +276,14 @@ internal double Phi(long timeDiff, double mean, double stdDeviation)
return -Math.Log10(1.0d - 1.0d/(1.0d + e));
}

private long MinStdDeviationMillis
private double MinStdDeviationMillis
{
get { return (long)_minStdDeviation.TotalMilliseconds; }
get { return _minStdDeviation.TotalMilliseconds; }
}

private long AcceptableHeartbeatPauseMillis
private double AcceptableHeartbeatPauseMillis
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove boxing back to long in order to prevent loss of precision.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(may not have been an issue anyway)

{
get { return (long)_acceptableHeartbeatPause.TotalMilliseconds; }
get { return _acceptableHeartbeatPause.TotalMilliseconds; }
}

private double EnsureValidStdDeviation(double stdDeviation)
Expand All @@ -300,20 +301,20 @@ private double EnsureValidStdDeviation(double stdDeviation)
/// The stats (mean, variance, stdDeviation) are not defined for empty
/// <see cref="HeartbeatHistory"/>, i.e. throws Exception
/// </summary>
internal class HeartbeatHistory
internal readonly struct HeartbeatHistory
{
private int _maxSampleSize;
private List<long> _intervals;
private long _intervalSum;
private long _squaredIntervalSum;
private readonly int _maxSampleSize;
private readonly ImmutableList<long> _intervals;
private readonly long _intervalSum;
private readonly long _squaredIntervalSum;

/// <summary>
/// TBD
/// Creates a new <see cref="HeartbeatHistory"/> instance.
/// </summary>
/// <param name="maxSampleSize">TBD</param>
/// <param name="intervals">TBD</param>
/// <param name="intervalSum">TBD</param>
/// <param name="squaredIntervalSum">TBD</param>
/// <param name="maxSampleSize">The maximum number of samples to retain. Older ones are dropped once intervals exceeds this value.</param>
/// <param name="intervals">The range of recorded time intervals.</param>
/// <param name="intervalSum">The sum of the recorded time intervals.</param>
/// <param name="squaredIntervalSum">The squared sum of the intervals.</param>
/// <exception cref="ArgumentOutOfRangeException">
/// This exception is thrown for the following reasons:
/// <ul>
Expand All @@ -322,7 +323,7 @@ internal class HeartbeatHistory
/// <li>The specified <paramref name="squaredIntervalSum"/> is less than zero.</li>
/// </ul>
/// </exception>
public HeartbeatHistory(int maxSampleSize, List<long> intervals, long intervalSum, long squaredIntervalSum)
public HeartbeatHistory(int maxSampleSize, ImmutableList<long> intervals, long intervalSum, long squaredIntervalSum)
{
_maxSampleSize = maxSampleSize;
_intervals = intervals;
Expand All @@ -337,29 +338,11 @@ public HeartbeatHistory(int maxSampleSize, List<long> intervals, long intervalSu
throw new ArgumentOutOfRangeException(nameof(squaredIntervalSum), $"squaredIntervalSum must be >= 0, got {squaredIntervalSum}");
}

/// <summary>
/// TBD
/// </summary>
public double Mean
{
get { return ((double)_intervalSum / _intervals.Count); }
}
public double Mean => ((double)_intervalSum / _intervals.Count);

/// <summary>
/// TBD
/// </summary>
public double Variance
{
get { return ((double)_squaredIntervalSum / _intervals.Count) - (Mean * Mean); }
}
public double Variance => ((double)_squaredIntervalSum / _intervals.Count) - (Mean * Mean);

/// <summary>
/// TBD
/// </summary>
public double StdDeviation
{
get { return Math.Sqrt(Variance); }
}
public double StdDeviation => Math.Sqrt(Variance);

/// <summary>
/// Increments the <see cref="HeartbeatHistory"/>.
Expand All @@ -371,7 +354,7 @@ public double StdDeviation
{
if (history._intervals.Count < history._maxSampleSize)
{
return new HeartbeatHistory(history._maxSampleSize, history._intervals.Concat(new[] { interval }).ToList(),
return new HeartbeatHistory(history._maxSampleSize, history._intervals.Add(interval),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use Systems.Collections.Immutable here instead of classic collections and LINQ (this is 7-8 year old code)

history._intervalSum + interval, history._squaredIntervalSum + Pow2(interval));
}
else
Expand All @@ -382,7 +365,8 @@ public double StdDeviation

private static HeartbeatHistory DropOldest(HeartbeatHistory history)
{
return new HeartbeatHistory(history._maxSampleSize, history._intervals.Skip(1).ToList(), history._intervalSum - history._intervals.First(), history._squaredIntervalSum - Pow2(history._intervals.First()));
return new HeartbeatHistory(history._maxSampleSize, history._intervals.RemoveAt(0), history._intervalSum - history._intervals.First(),
history._squaredIntervalSum - Pow2(history._intervals.First()));
}

private static long Pow2(long x)
Expand All @@ -398,11 +382,11 @@ private static long Pow2(long x)
/// The stats (mean, variance, stdDeviation) are not defined for empty
/// HeartbeatHistory and will throw DivideByZero exceptions
/// </summary>
/// <param name="maxSampleSize">TBD</param>
/// <returns>TBD</returns>
/// <param name="maxSampleSize">The maximum number of samples to include in this history.</param>
/// <returns>A new <see cref="HeartbeatHistory"/> instance.</returns>
public static HeartbeatHistory Apply(int maxSampleSize)
{
return new HeartbeatHistory(maxSampleSize, new List<long>(), 0L, 0L);
return new HeartbeatHistory(maxSampleSize, ImmutableList<long>.Empty, 0L, 0L);
}

#endregion
Expand Down