-
Notifications
You must be signed in to change notification settings - Fork 1k
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.
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
TBD comments and local function support in Akka core #2697
TBD comments and local function support in Akka core #2697
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commenting on my changes.
Func<List<string>, Task<Done>> loop = null; | ||
loop = remainingPhases => | ||
|
||
Task<Done> Loop(List<string> remainingPhases) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Local function optimization in CoordinatedShutdown
, which should have minimal impact if any.
@@ -483,26 +481,24 @@ internal static List<string> TopologicalSort(Dictionary<string, Phase> phases) | |||
var unmarked = new HashSet<string>(phases.Keys.Concat(phases.Values.SelectMany(x => x.DependsOn))); | |||
var tempMark = new HashSet<string>(); // for detecting cycles | |||
|
|||
Action<string> depthFirstSearch = null; | |||
depthFirstSearch = u => | |||
void DepthFirstSearch(string u) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another local function optimization in CoordinatedShutdown
. Minimal impact - this code only runs once per application.
@@ -375,8 +375,7 @@ private void ProcessMailbox(int left, long deadlineTicks) | |||
{ | |||
while (ShouldProcessMessage()) | |||
{ | |||
Envelope next; | |||
if (!TryDequeue(out next)) return; | |||
if (!TryDequeue(out var next)) return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed mailbox to use new out var
syntax. Don't expect this to have any real performance impact.
@@ -277,38 +277,36 @@ public MailboxType GetMailboxType(Props props, Config dispatcherConfig) | |||
_mailboxSizeWarningIssued = true; | |||
} | |||
|
|||
Func<MailboxType, MailboxType> verifyRequirements = mailboxType => | |||
MailboxType VerifyRequirements(MailboxType mailboxType) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be interesting - if the VerifyRequirements
local function is able to be created using a value type, which the compiler tries to do by default, this might improve the rate at which we're able to spawn actors. Will check benchmark results to verify.
@Aaronontheweb Where is a performance comparison? |
BeforeAkka.Tests.Performance.Actor.ActorMemoryFootprintSpec+ReceiveActor_memory_footprintMeasures the amount of memory used by 10,000 ReceiveActors System InfoNBench=NBench, Version=1.0.1.0, Culture=neutral, PublicKeyToken=null
OS=Microsoft Windows NT 6.2.9200.0
ProcessorCount=2
CLR=4.0.30319.42000,IsMono=False,MaxGcGeneration=2 NBench SettingsRunMode=Iterations, TestMode=Measurement
NumberOfIterations=13, MaximumRunTime=00:00:01
Concurrent=True
Tracing=True DataTotals
Per-second Totals
|
AfterAkka.Tests.Performance.Actor.ActorMemoryFootprintSpec+ReceiveActor_memory_footprintMeasures the amount of memory used by 10,000 ReceiveActors System InfoNBench=NBench, Version=1.0.1.0, Culture=neutral, PublicKeyToken=null
OS=Microsoft Windows NT 6.2.9200.0
ProcessorCount=2
CLR=4.0.30319.42000,IsMono=False,MaxGcGeneration=2 NBench SettingsRunMode=Iterations, TestMode=Measurement
NumberOfIterations=13, MaximumRunTime=00:00:01
Concurrent=True
Tracing=True DataTotals
Per-second Totals
|
@alexvaluyskiy ActorCreateThroughput is virtually the same, but this benchmark may not be a reliable measure for any perf changes here since the actor type we're creating is the same each time. The JITer can optimize the code after warm-ups to re-use the same mailbox type and type check each time since it's always the same actor and same mailbox type. For the same reasons I mentioned on the other PR though, there's only upside to moving away from delegates to local functions here. Worst case scenario gets implemented the same way we've done it with |
Another plus is that local functions are slightly easier and natural to read than Func<>. |
* added comments to internals * updated intellisense comments * added comments for systemmessage * added more local function support
No description provided.