Remoting system upgrade #1596

Aaronontheweb · 2015-12-21T03:11:39Z

Opening a streaming pull request for a set of changes I'm working on:

Converting all system remoting actors to ReceiveActor to make them easier to reason about, with exception of the ProtocolStateActor FSM.
Updating RemotingSettings and Remote.conf to support some new values.
Upgrading the ReliableDeliverySupervisor and fixing the backoff and buffering mechanism.
Updating the EndpointManager, EndpointWriter, and EndpointReader where needed.

rogeralsing · 2015-12-21T05:34:49Z

Much better readability on the ReceiveActors.
When this is done and before we pull it in, I think we should do some extensive memory testing around the receive actors.
I have seen some memory retention related to them and I currently don't know if it is only for the first time compilation of each use (the runtime compile a message receive handler from the lambdas)
Or if they might actually do leak and recompile for each restart.

Hopefully there is no problem but there is unknown memory retention ATM.

Aaronontheweb · 2015-12-29T18:14:43Z

@rogeralsing

Or if they might actually do leak and recompile for each restart.

I wonder if behavior-switching could be a culprit here too?

Aaronontheweb · 2015-12-29T19:23:46Z

Have a new test failure around the ThrottleTransportAdapter now as a result of some of the changes I made to EndpointWriter - looking into it

Aaronontheweb · 2015-12-31T01:55:24Z

This is finished. Going to comment on it in detail now to make it easier to review.

Aaronontheweb · 2015-12-31T01:57:32Z

src/core/Akka.Remote.Tests.Performance/InboundMessageDispatcherSpec.cs

@@ -32,9 +32,11 @@ private class BenchmarkActorRef : MinimalActorRef
        {
            private readonly Counter _counter;

-            public BenchmarkActorRef(Counter counter)
+            public BenchmarkActorRef(Counter counter, RemoteActorRefProvider provider)


Fixing some bugs here that I introduced when I changed the way IInboundMessageDispatcher works - specifically, I changed the way two IActorRefs compare equality. Instead of using == they now use .Equals, which is correct. Unfortunately that caused this code to throw a NullReferenceException during the performance test due to the Path property of BenchmarkActorRef being null. These changes fix that.

👍
(we should really go through all of our code to find == on actorrefs, I do think we have a few of those. but that is for another issue/task)

Aaronontheweb · 2016-01-05T22:13:05Z

src/core/Akka.Remote/EndpointManager.cs

+                                        result.Exception.Handle(e => true);
+                                    return false;
+                                }
+                                return result.Result.All(x => x);


Otherwise, && up all of the individual statuses. All pass or none do.

Aaronontheweb · 2016-01-05T23:17:07Z

This PR is good to go - I've reviewed everything extensively and commented on each part that does anything significant, including parts that haven't really been modified as part of this PR.

cc @akkadotnet/contributors could someone take a look through my comments and do a sanity check? I think it's ready for merge once the tests pass.

Aaronontheweb · 2016-01-05T23:18:25Z

Ah, I almost forgot - we need to do a benchmark comparison. Will include that in comments once we have latest from build server.

Aaronontheweb · 2016-01-06T03:01:30Z

Performance from this build:

Akka.Remote.Tests.Performance.Transports.TestTransportRemoteMessagingThroughputSpec+OneWay

Measures the throughput of Akka.Remote over a particular transport using one-way messaging
1/6/2016 2:35:50 AM

System Info

NBench=NBench, Version=0.1.5.0, Culture=neutral, PublicKeyToken=null
OS=Microsoft Windows NT 6.2.9200.0
ProcessorCount=4
CLR=4.0.30319.42000,IsMono=False,MaxGcGeneration=2
WorkerThreads=32767, IOThreads=4

NBench Settings

RunMode=Iterations, TestMode=Measurement
NumberOfIterations=13, MaximumRunTime=00:00:01

Data

Totals

Metric	Units	Max	Average	Min	StdDev
TotalCollections [Gen0]	collections	23.00	22.69	22.00	0.48
TotalCollections [Gen1]	collections	1.00	0.77	0.00	0.44
TotalCollections [Gen2]	collections	0.00	0.00	0.00	0.00
[Counter] RemoteMessageReceived	operations	10,000.00	10,000.00	10,000.00	0.00

Per-second Totals

Metric	Units / s	Max / s	Average / s	Min / s	StdDev / s
TotalCollections [Gen0]	collections	20.68	19.58	18.17	0.68
TotalCollections [Gen1]	collections	0.90	0.66	0.00	0.38
TotalCollections [Gen2]	collections	0.00	0.00	0.00	0.00
[Counter] RemoteMessageReceived	operations	9,007.89	8,633.82	7,898.97	364.55

Raw Data

(Removed the GC raw data for readability)

[Counter] RemoteMessageReceived

Run #	operations	operations / s	ns / operations
1	10,000.00	8,887.42	112,518.57
2	10,000.00	8,015.63	124,756.31
3	10,000.00	9,007.89	111,013.73
4	10,000.00	7,898.97	126,598.77
5	10,000.00	9,006.66	111,028.97
6	10,000.00	8,553.62	116,909.62
7	10,000.00	8,570.38	116,680.95
8	10,000.00	8,857.40	112,899.90
9	10,000.00	8,989.97	111,235.12
10	10,000.00	8,671.31	115,322.81
11	10,000.00	8,678.47	115,227.66
12	10,000.00	8,301.06	120,466.54
13	10,000.00	8,800.82	113,625.75

Aaronontheweb · 2016-01-06T03:08:38Z

From the latest dev branch:

Akka.Remote.Tests.Performance.Transports.TestTransportRemoteMessagingThroughputSpec+OneWay

Measures the throughput of Akka.Remote over a particular transport using one-way messaging
1/6/2016 2:57:47 AM

System Info

NBench=NBench, Version=0.1.5.0, Culture=neutral, PublicKeyToken=null
OS=Microsoft Windows NT 6.2.9200.0
ProcessorCount=4
CLR=4.0.30319.42000,IsMono=False,MaxGcGeneration=2
WorkerThreads=32767, IOThreads=4

NBench Settings

RunMode=Iterations, TestMode=Measurement
NumberOfIterations=13, MaximumRunTime=00:00:01

Data

Totals

Metric	Units	Max	Average	Min	StdDev
TotalCollections [Gen0]	collections	25.00	25.00	25.00	0.00
TotalCollections [Gen1]	collections	1.00	0.69	0.00	0.48
TotalCollections [Gen2]	collections	0.00	0.00	0.00	0.00
[Counter] RemoteMessageReceived	operations	10,000.00	10,000.00	10,000.00	0.00

Per-second Totals

Metric	Units / s	Max / s	Average / s	Min / s	StdDev / s
TotalCollections [Gen0]	collections	17.44	15.99	14.32	0.91
TotalCollections [Gen1]	collections	0.70	0.44	0.00	0.31
TotalCollections [Gen2]	collections	0.00	0.00	0.00	0.00
[Counter] RemoteMessageReceived	operations	6,975.93	6,396.60	5,726.39	363.80

Raw Data

(Removed the GC raw data for readability)

[Counter] RemoteMessageReceived

Run #	operations	operations / s	ns / operations
1	10,000.00	6,060.10	165,013.70
2	10,000.00	6,798.06	147,100.85
3	10,000.00	5,726.39	174,629.95
4	10,000.00	6,975.93	143,350.05
5	10,000.00	6,506.92	153,682.58
6	10,000.00	6,332.67	157,911.38
7	10,000.00	6,019.34	166,131.07
8	10,000.00	6,386.24	156,586.77
9	10,000.00	6,492.86	154,015.28
10	10,000.00	6,680.32	149,693.41
11	10,000.00	6,383.50	156,653.98
12	10,000.00	6,771.18	147,684.78
13	10,000.00	6,022.30	166,049.46

Aaronontheweb · 2016-01-06T03:09:20Z

Looks like an increase of almost 2000 msg / s. I can try running the benchmark multiple times again to eliminate any noise that a specific VM might have created.

brings Akka.Remote up to code with latest JVM stable converted all Akka.Remote system actors to ReceiveActor fixed race condition in Akka.Remote.Tests.Performance

Remoting system upgrade

Aaronontheweb added the needs review label Dec 21, 2015

rogeralsing added reviewed with remarks and removed needs review labels Dec 29, 2015

Aaronontheweb force-pushed the remoting-actors-receiveactor branch from 616c8fb to aa62dbc Compare December 29, 2015 18:13

Aaronontheweb force-pushed the remoting-actors-receiveactor branch from 1aed22c to f251d14 Compare December 31, 2015 01:54

Aaronontheweb changed the title ~~[WIP] Remoting system upgrade~~ Remoting system upgrade Dec 31, 2015

Aaronontheweb added the needs review label Dec 31, 2015

Aaronontheweb reviewed Dec 31, 2015
View reviewed changes

Aaronontheweb reviewed Jan 5, 2016
View reviewed changes

Aaronontheweb added the comments by author label Jan 5, 2016

Aaronontheweb force-pushed the remoting-actors-receiveactor branch from 193a62e to 9e8f7be Compare January 5, 2016 23:14

close akkadotnet#1586

b642984

brings Akka.Remote up to code with latest JVM stable converted all Akka.Remote system actors to ReceiveActor fixed race condition in Akka.Remote.Tests.Performance

Aaronontheweb force-pushed the remoting-actors-receiveactor branch from 9e8f7be to b642984 Compare January 6, 2016 19:38

Horusiath added a commit that referenced this pull request Jan 7, 2016

Merge pull request #1596 from Aaronontheweb/remoting-actors-receiveactor

d14ad1e

Remoting system upgrade

Horusiath merged commit d14ad1e into akkadotnet:dev Jan 7, 2016

skotzko removed the needs review label Jan 7, 2016

Aaronontheweb mentioned this pull request Jan 8, 2016

Member not being removed to allow new incarnation of that member to join. #1458

Closed

This was referenced Jan 18, 2016

Akka.NET v1.0.6 release (DEV) #1653

Merged

Akka.NET v1.0.6 release (PRODUCTION) #1654

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remoting system upgrade #1596

Remoting system upgrade #1596

Aaronontheweb commented Dec 21, 2015

rogeralsing commented Dec 21, 2015

Aaronontheweb commented Dec 29, 2015

Aaronontheweb commented Dec 29, 2015

Aaronontheweb commented Dec 31, 2015

Aaronontheweb Dec 31, 2015

rogeralsing Dec 31, 2015

Aaronontheweb Jan 5, 2016

Aaronontheweb commented Jan 5, 2016

Aaronontheweb commented Jan 5, 2016

Aaronontheweb commented Jan 6, 2016

Aaronontheweb commented Jan 6, 2016

Aaronontheweb commented Jan 6, 2016

Remoting system upgrade #1596

Remoting system upgrade #1596

Conversation

Aaronontheweb commented Dec 21, 2015

rogeralsing commented Dec 21, 2015

Aaronontheweb commented Dec 29, 2015

Aaronontheweb commented Dec 29, 2015

Aaronontheweb commented Dec 31, 2015

Aaronontheweb Dec 31, 2015

Choose a reason for hiding this comment

rogeralsing Dec 31, 2015

Choose a reason for hiding this comment

Aaronontheweb Jan 5, 2016

Choose a reason for hiding this comment

Aaronontheweb commented Jan 5, 2016

Aaronontheweb commented Jan 5, 2016

Aaronontheweb commented Jan 6, 2016

Akka.Remote.Tests.Performance.Transports.TestTransportRemoteMessagingThroughputSpec+OneWay

System Info

NBench Settings

Data

Totals

Per-second Totals

Raw Data

[Counter] RemoteMessageReceived

Aaronontheweb commented Jan 6, 2016

Akka.Remote.Tests.Performance.Transports.TestTransportRemoteMessagingThroughputSpec+OneWay

System Info

NBench Settings

Data

Totals

Per-second Totals

Raw Data

[Counter] RemoteMessageReceived

Aaronontheweb commented Jan 6, 2016