Skip to content

Commit fc37902

Browse files
jhicksonjhickson
authored
Improve actor performance via simplication (#26)
Changes related to performance: - Removed state machine in ActorImpl and merged ActorImpl into Actor; - Refined ActorTaskScheduler and the way the kind of task being executed is communicated so that using async state is no longer necessary; - Replaced use of Queue<> in ActorTaskScheduler with something that easily allows putting something at the front, which simplified some logic; - Made use of pause/resume functionality in ActorTaskScheduler to simplify starting an Actor; - Made locking in ActorTaskScheduler more fine-grained; - Used UnsafeQueueUserWorkItem on platforms that support it. Also: - Added NET Standard 2.0 support; - Fixed various long-standing bugs that appeared in edge cases. Overall, the changes made have reduced locking and complexity, which, apart from anything, has made it easier to track down long-standing spurious test failures and bugs. Performance isn't monumentally better but confidence that things are working correctly has been increased somewhat.
1 parent e580f87 commit fc37902

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+1017
-1583
lines changed

‎.travis.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ addons:
2020
key_url: 'https://packages.microsoft.com/keys/microsoft.asc'
2121
packages:
2222
- dotnet-hostfxr-1.0.1
23-
- dotnet-sharedframework-microsoft.netcore.app-1.0.9
23+
- dotnet-sharedframework-microsoft.netcore.app-1.1.6
2424

2525
script:
2626
- (git fetch --unshallow 2>/dev/null || echo "Already full repo.")

‎Winton.Extensions.Threading.Actor.Tests.Unit/ActorTests.cs‎

Lines changed: 103 additions & 142 deletions
Large diffs are not rendered by default.

‎Winton.Extensions.Threading.Actor.Tests.Unit/Internal/ActorTaskSchedulerTests.cs‎

Lines changed: 279 additions & 171 deletions
Large diffs are not rendered by default.

‎Winton.Extensions.Threading.Actor.Tests.Unit/Internal/ActorWorkSchedulerTests.cs‎

Lines changed: 71 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,8 @@
44
using System;
55
using System.Collections.Generic;
66
using System.Linq;
7-
using System.Threading;
87
using System.Threading.Tasks;
9-
using Moq;
108
using FluentAssertions;
11-
using Winton.Extensions.Threading.Actor.Internal;
129
using Winton.Extensions.Threading.Actor.Tests.Utilities;
1310
using Xunit;
1411

@@ -24,20 +21,18 @@ public enum WorkType
2421

2522
private readonly IActor _actor;
2623
private readonly IActorWorkScheduler _scheduler;
27-
private readonly IActorTaskFactory _actorTaskFactory;
2824

2925
public ActorWorkSchedulerTests()
3026
{
31-
_actorTaskFactory = SetUpTaskFactory();
32-
_actor = new Actor(_actorTaskFactory);
27+
_actor = new Actor();
3328
_actor.Start();
34-
_scheduler = new ActorWorkScheduler(_actor, _actorTaskFactory);
29+
_scheduler = _actor.CreateWorkScheduler();
3530
}
3631

3732
public void Dispose()
3833
{
34+
_actor.Stop().Wait();
3935
_scheduler.CancelCurrent();
40-
_actor.Stop();
4136
}
4237

4338
[Theory]
@@ -52,31 +47,30 @@ public void ShouldBeAbleToScheduleWorkToRepeatAtAFixedInterval(WorkType workType
5247
var times = new List<DateTime>();
5348
var sampleSize = 5;
5449

55-
Action adder =
56-
() =>
50+
void Adder()
51+
{
52+
if (times.Count < sampleSize)
5753
{
58-
if (times.Count < sampleSize)
59-
{
60-
times.Add(DateTime.UtcNow);
61-
}
62-
63-
if (times.Count == sampleSize)
64-
{
65-
// Block here so that we can assess something that's not moving
66-
barrier.Task.Wait();
67-
}
68-
};
54+
times.Add(DateTime.UtcNow);
55+
}
56+
57+
if (times.Count == sampleSize)
58+
{
59+
// Block here so that we can assess something that's not moving
60+
barrier.Task.Wait();
61+
}
62+
}
6963

7064
switch (workType)
7165
{
7266
case WorkType.Sync:
73-
_scheduler.Schedule(adder, expectedInterval, actorScheduleOptions);
67+
_scheduler.Schedule((Action)Adder, expectedInterval, actorScheduleOptions);
7468
break;
7569
case WorkType.Async:
7670
_scheduler.Schedule(async () =>
7771
{
7872
await Task.Yield();
79-
adder();
73+
Adder();
8074
},
8175
expectedInterval,
8276
actorScheduleOptions);
@@ -92,35 +86,53 @@ public void ShouldBeAbleToScheduleWorkToRepeatAtAFixedInterval(WorkType workType
9286
// Use 75ms instead of 100ms to give it a bit of latitude: mainly we just want to make sure there is some delaying going on.
9387
actualIntervals.Should().OnlyContain(x => x >= TimeSpan.FromMilliseconds(75));
9488

95-
var expectedNumberOfDelays = sampleSize - (actorScheduleOptions.HasFlag(ActorScheduleOptions.NoInitialDelay) ? 1 : 0);
96-
97-
Expect.That(() => Mock.Get(_actorTaskFactory)
98-
.Verify(x => x.CreateDelay(expectedInterval, It.IsAny<CancellationToken>()), Times.Exactly(expectedNumberOfDelays)))
99-
.ShouldNotThrow();
100-
10189
barrier.SetResult(true);
10290
}
10391

10492
[Fact]
10593
public void ShouldBeAbleToSpecifyThatSyncWorkIsLongRunning()
10694
{
107-
_scheduler.Schedule(() => { }, TimeSpan.FromMilliseconds(1000), ActorScheduleOptions.NoInitialDelay | ActorScheduleOptions.WorkIsLongRunning);
108-
Within.FiveSeconds(
109-
() => Expect.That(
110-
() => Mock.Get(_actorTaskFactory)
111-
.Verify(x => x.Create(It.IsAny<Action<object>>(), It.IsAny<CancellationToken>(), It.Is<TaskCreationOptions>(o => o.HasFlag(TaskCreationOptions.LongRunning)), It.IsAny<object>()), Times.Once))
112-
.ShouldNotThrow());
95+
var taskCompletionSource = new TaskCompletionSource<object>();
96+
97+
_scheduler.Schedule(
98+
() =>
99+
{
100+
try
101+
{
102+
ActorThreadAssertions.CurrentThreadShouldNotBeThreadPoolThread();
103+
taskCompletionSource.TrySetResult(null);
104+
}
105+
catch (Exception exception)
106+
{
107+
taskCompletionSource.TrySetException(exception);
108+
}
109+
}, TimeSpan.FromMilliseconds(1000), ActorScheduleOptions.NoInitialDelay | ActorScheduleOptions.WorkIsLongRunning);
110+
111+
taskCompletionSource.Awaiting(async x => await x.Task).ShouldNotThrow();
113112
}
114113

115114
[Fact]
116115
public void ShouldBeAbleToSpecifyThatAsyncWorkIsLongRunning()
117116
{
118-
_scheduler.Schedule(async () => { await Task.Yield(); }, TimeSpan.FromMilliseconds(1000), ActorScheduleOptions.NoInitialDelay | ActorScheduleOptions.WorkIsLongRunning);
119-
Within.FiveSeconds(
120-
() => Expect.That(
121-
() => Mock.Get(_actorTaskFactory)
122-
.Verify(x => x.Create(It.IsAny<Func<object, Task>>(), It.IsAny<CancellationToken>(), It.Is<TaskCreationOptions>(o => o.HasFlag(TaskCreationOptions.LongRunning)), It.IsAny<object>()), Times.Once))
123-
.ShouldNotThrow());
117+
var taskCompletionSource = new TaskCompletionSource<object>();
118+
119+
_scheduler.Schedule(
120+
async () =>
121+
{
122+
await Task.Yield();
123+
124+
try
125+
{
126+
ActorThreadAssertions.CurrentThreadShouldNotBeThreadPoolThread();
127+
taskCompletionSource.TrySetResult(null);
128+
}
129+
catch (Exception exception)
130+
{
131+
taskCompletionSource.TrySetException(exception);
132+
}
133+
}, TimeSpan.FromMilliseconds(1000), ActorScheduleOptions.NoInitialDelay | ActorScheduleOptions.WorkIsLongRunning);
134+
135+
taskCompletionSource.Awaiting(async x => await x.Task).ShouldNotThrow();
124136
}
125137

126138
[Theory]
@@ -172,31 +184,30 @@ public void ASecondCallToScheduleShouldCancelTheWorkPreviouslyScheduled(WorkType
172184
var firstTwoAddedPromise = new TaskCompletionSource<bool>();
173185
var gotAOneAfterATwoPromise = new TaskCompletionSource<bool>();
174186

175-
Action<string> adder =
176-
x =>
177-
{
178-
output.Add(x);
187+
void Adder(string x)
188+
{
189+
output.Add(x);
179190

180-
if (x == "two")
181-
{
182-
firstTwoAddedPromise.TrySetResult(true);
183-
}
184-
else if (firstTwoAddedPromise.Task.IsCompleted && x == "one")
185-
{
186-
gotAOneAfterATwoPromise.TrySetResult(true);
187-
}
188-
};
191+
if (x == "two")
192+
{
193+
firstTwoAddedPromise.TrySetResult(true);
194+
}
195+
else if (firstTwoAddedPromise.Task.IsCompleted && x == "one")
196+
{
197+
gotAOneAfterATwoPromise.TrySetResult(true);
198+
}
199+
}
189200

190201
switch (workType1)
191202
{
192203
case WorkType.Sync:
193-
task1 = _scheduler.Schedule(() => { adder("one"); }, interval);
204+
task1 = _scheduler.Schedule(() => { Adder("one"); }, interval);
194205
break;
195206
case WorkType.Async:
196207
task1 = _scheduler.Schedule(async () =>
197208
{
198-
await Task.Yield();
199-
adder("one");
209+
await Task.Delay(TimeSpan.FromMilliseconds(10));
210+
Adder("one");
200211
}, interval);
201212
break;
202213
default:
@@ -208,12 +219,12 @@ public void ASecondCallToScheduleShouldCancelTheWorkPreviouslyScheduled(WorkType
208219
switch (workType2)
209220
{
210221
case WorkType.Sync:
211-
_scheduler.Schedule(() => { adder("two"); }, interval);
222+
_scheduler.Schedule(() => { Adder("two"); }, interval);
212223
break;
213224
case WorkType.Async:
214225
_scheduler.Schedule(async () =>
215226
{
216-
adder("two");
227+
Adder("two");
217228
await Task.Yield();
218229
}, interval);
219230
break;
@@ -348,40 +359,9 @@ public void AsynchronousSchedulerExtensionShouldEmitAnyArgumentOutOfRangeExcepti
348359
[Fact]
349360
public void AsynchronousSchedulerExtensionShouldEmitAnyArgumentNullExceptions()
350361
{
351-
Expect.That(() => _scheduler.Schedule((Func<Task>)null, TimeSpan.FromDays(1), ActorScheduleOptions.Default, x => { }))
362+
Expect.That(() => _scheduler.Schedule(null, TimeSpan.FromDays(1), ActorScheduleOptions.Default, x => { }))
352363
.ShouldThrow<ArgumentNullException>()
353364
.And.ParamName.Should().Be("work");
354365
}
355-
356-
private static IActorTaskFactory SetUpTaskFactory()
357-
{
358-
var realTaskFactory = new ActorTaskFactory();
359-
var taskFactory = Mock.Of<IActorTaskFactory>();
360-
361-
Mock.Get(taskFactory)
362-
.Setup(x => x.Create(It.IsAny<Action<object>>(), It.IsAny<CancellationToken>(), It.IsAny<TaskCreationOptions>(), It.IsAny<object>()))
363-
.Returns<Action<object>, CancellationToken, TaskCreationOptions, object>((action, cancellationToken, taskCreationOptions, state) => realTaskFactory.Create(action, cancellationToken, taskCreationOptions, state));
364-
Mock.Get(taskFactory)
365-
.Setup(x => x.Create(It.IsAny<Func<object, int>>(), It.IsAny<CancellationToken>(), It.IsAny<TaskCreationOptions>(), It.IsAny<object>()))
366-
.Returns<Func<object, int>, CancellationToken, TaskCreationOptions, object>((function, cancellationToken, taskCreationOptions, state) => realTaskFactory.Create(function, cancellationToken, taskCreationOptions, state));
367-
Mock.Get(taskFactory)
368-
.Setup(x => x.Create(It.IsAny<Func<object, Task>>(), It.IsAny<CancellationToken>(), It.IsAny<TaskCreationOptions>(), It.IsAny<object>()))
369-
.Returns<Func<object, Task>, CancellationToken, TaskCreationOptions, object>((function, cancellationToken, taskCreationOptions, state) => realTaskFactory.Create(function, cancellationToken, taskCreationOptions, state));
370-
Mock.Get(taskFactory)
371-
.Setup(x => x.Create(It.IsAny<Func<object, Task<string>>>(), It.IsAny<CancellationToken>(), It.IsAny<TaskCreationOptions>(), It.IsAny<object>()))
372-
.Returns<Func<object, Task<string>>, CancellationToken, TaskCreationOptions, object>((function, cancellationToken, taskCreationOptions, state) => realTaskFactory.Create(function, cancellationToken, taskCreationOptions, state));
373-
Mock.Get(taskFactory)
374-
.Setup(x => x.FromCompleted())
375-
.Returns(realTaskFactory.FromCompleted);
376-
Mock.Get(taskFactory)
377-
.Setup(x => x.FromException(It.IsAny<Exception>()))
378-
.Returns<Exception>(realTaskFactory.FromException);
379-
Mock.Get(taskFactory)
380-
.Setup(x => x.CreateDelay(It.IsAny<TimeSpan>(), It.IsAny<CancellationToken>()))
381-
.Returns<TimeSpan, CancellationToken>(Task.Delay);
382-
383-
return taskFactory;
384-
}
385-
386366
}
387367
}

‎Winton.Extensions.Threading.Actor.Tests.Unit/Winton.Extensions.Threading.Actor.Tests.Unit.csproj‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
<Project Sdk="Microsoft.NET.Sdk">
22

33
<PropertyGroup>
4-
<TargetFrameworks>netcoreapp1.0;net462</TargetFrameworks>
4+
<TargetFrameworks>netcoreapp1.1;netcoreapp2.0;net462</TargetFrameworks>
55
<SignAssembly>True</SignAssembly>
66
<DelaySign>False</DelaySign>
77
<AssemblyOriginatorKeyFile>../keys/Winton.Extensions.Threading.Actor.snk</AssemblyOriginatorKeyFile>
88
<PublicSign Condition=" '$(OS)' != 'Windows_NT' ">true</PublicSign>
9+
<LangVersion>latest</LangVersion>
910
</PropertyGroup>
1011

1112
<ItemGroup>
12-
<PackageReference Include="FluentAssertions" Version="4.19.4" />
1313
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.5.0" />
1414
<PackageReference Include="Moq" Version="4.8.1" />
1515
<PackageReference Include="xunit" Version="2.3.1" />

‎Winton.Extensions.Threading.Actor.Tests.Unit/Winton.Extensions.Threading.Actor.Tests.Unit.csproj.DotSettings‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
<wpf:ResourceDictionary xml:space="preserve" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib" xmlns:ss="urn:shemas-jetbrains-com:settings-storage-xaml" xmlns:wpf="http://schemas.microsoft.com/winfx/2006/xaml/presentation">
2+
<s:String x:Key="/Default/CodeInspection/CSharpLanguageProject/LanguageLevel/@EntryValue">CSharp71</s:String></wpf:ResourceDictionary>

‎Winton.Extensions.Threading.Actor.Tests.Utilities/ActorThreadAssertions.cs‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
using System.Threading;
2+
using FluentAssertions;
3+
4+
namespace Winton.Extensions.Threading.Actor.Tests.Utilities
5+
{
6+
public static class ActorThreadAssertions
7+
{
8+
public static void CurrentThreadShouldBeThreadPoolThread()
9+
{
10+
#if !NETSTANDARD1_3
11+
Thread.CurrentThread.IsThreadPoolThread.Should().BeTrue("should be on thread pool thread");
12+
#endif
13+
Thread.CurrentThread.Name.Should().NotBe("ActorWorker", "should be on thread pool thread");
14+
}
15+
16+
public static void CurrentThreadShouldNotBeThreadPoolThread()
17+
{
18+
#if !NETSTANDARD1_3
19+
Thread.CurrentThread.IsThreadPoolThread.Should().BeFalse("should not be on thread pool thread");
20+
#endif
21+
Thread.CurrentThread.Name.Should().Be("ActorWorker", "should not be on thread pool thread");
22+
}
23+
}
24+
}

‎Winton.Extensions.Threading.Actor.Tests.Utilities/Winton.Extensions.Threading.Actor.Tests.Utilities.csproj‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
<PropertyGroup>
44
<Copyright>Copyright © 2017 Winton</Copyright>
55
<Authors>Jos Hickson</Authors>
6-
<TargetFrameworks>net451;netstandard1.3</TargetFrameworks>
6+
<TargetFrameworks>net451;netstandard1.3;netstandard2.0</TargetFrameworks>
77
<NoWarn>$(NoWarn);CS1591</NoWarn>
88
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
99
<AssemblyName>Winton.Extensions.Threading.Actor.Tests.Utilities</AssemblyName>
@@ -29,14 +29,8 @@
2929
<DebugType>full</DebugType>
3030
</PropertyGroup>
3131

32-
<ItemGroup>
33-
<Folder Include="Properties\" />
34-
</ItemGroup>
35-
36-
<ItemGroup Condition="'$(TargetFramework)' == 'netstandard1.3'">
37-
<PackageReference Include="System.Threading.Thread">
38-
<Version>4.3.0</Version>
39-
</PackageReference>
32+
<ItemGroup Condition="'$(TargetFramework)' != 'net451'">
33+
<PackageReference Include="System.Threading.Thread" Version="4.3.0" />
4034
</ItemGroup>
4135

4236
</Project>

‎Winton.Extensions.Threading.Actor.sln‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11

22
Microsoft Visual Studio Solution File, Format Version 12.00
33
# Visual Studio 15
4-
VisualStudioVersion = 15.0.26730.10
4+
VisualStudioVersion = 15.0.27130.2010
55
MinimumVisualStudioVersion = 10.0.40219.1
66
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Libraries", "Libraries", "{F9C63802-0833-4F6F-A220-3193ACA0FC67}"
77
EndProject

0 commit comments

Comments
 (0)