Skip to content
This repository has been archived by the owner on Aug 2, 2023. It is now read-only.

Scheduler improvements #2026

Merged
merged 5 commits into from
Jan 11, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
2 changes: 1 addition & 1 deletion src/System.IO.Pipelines/System.IO.Pipelines.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<Import Project="..\..\tools\common.props" />
<PropertyGroup>
<Description>An abstraction for doing efficient asynchronous IO</Description>
<TargetFramework>netstandard1.1</TargetFramework>
<TargetFrameworks>netstandard1.1;netstandard2.0;netcoreapp2.1</TargetFrameworks>
Copy link
Member

Choose a reason for hiding this comment

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

This is probably the most unfortunate part of this change but meh..

Copy link
Member

Choose a reason for hiding this comment

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

Meh indeed. It's in implementation details with zero bearings on the user's experience.

Copy link
Member

Choose a reason for hiding this comment

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

It's in implementation details with zero bearings on the user's experience.

In this case yes.

<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<PackageIconUrl>http://go.microsoft.com/fwlink/?linkid=833199</PackageIconUrl>
<!--<DefineConstants Condition="'$(Configuration)' == 'Debug'">$(DefineConstants);BLOCK_LEASE_TRACKING</DefineConstants>-->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

namespace System.Threading
{
internal class InlineScheduler : Scheduler
internal sealed class InlineScheduler : Scheduler
{
public override void Schedule(Action action)
{
Expand Down
7 changes: 5 additions & 2 deletions src/System.IO.Pipelines/System/Threading/Scheduler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@ namespace System.Threading
{
public abstract class Scheduler
{
public static Scheduler TaskRun { get; } = new TaskRunScheduler();
Copy link
Member

Choose a reason for hiding this comment

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

Why this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because you said so here #2026 (review)

Copy link
Member

Choose a reason for hiding this comment

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

Should have been more specific, why the syntax change.

Copy link
Member Author

@benaadams benaadams Jan 11, 2018

Choose a reason for hiding this comment

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

Storing it in a Scheduler field means the exact type gets lost for:

Scheduler.ThreadPool.Schedule((() => {})

So it remains a virtual call.

Storing it in an exact type field ThreadPoolScheduler means when Scheduler.ThreadPool inlines the Jit knows exactly what it is and can devitalize it to a direct call.

Won't directly help pipelines as the Pipe stores the configured values in a Scheduler field; but will help anyone calling it directly.

public static Scheduler Inline { get; } = new InlineScheduler();
private static TaskRunScheduler _taskRunScheduler = new TaskRunScheduler();
private static InlineScheduler _inlineScheduler = new InlineScheduler();

public static Scheduler TaskRun => _taskRunScheduler;
Copy link
Member

Choose a reason for hiding this comment

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

ThreadPool

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

public static Scheduler Inline => _inlineScheduler;

public abstract void Schedule(Action action);
public abstract void Schedule(Action<object> action, object state);
Expand Down
64 changes: 63 additions & 1 deletion src/System.IO.Pipelines/System/Threading/TaskRunScheduler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,78 @@

namespace System.Threading
{
internal class TaskRunScheduler : Scheduler
internal sealed class TaskRunScheduler : Scheduler
{
public override void Schedule(Action action)
{
#if NETCOREAPP2_1
// Queue to low contention local ThreadPool queue; rather than global queue as per Task
ThreadPool.QueueUserWorkItem(_actionAsTask, action, preferLocal: true);
#elif NETSTANDARD2_0
ThreadPool.QueueUserWorkItem(_actionAsTask, action);
#else
Task.Factory.StartNew(action);
#endif
}

public override void Schedule(Action<object> action, object state)
{
#if NETCOREAPP2_1
// Queue to low contention local ThreadPool queue; rather than global queue as per Task
ThreadPool.QueueUserWorkItem(_actionObjectAsTask, new ActionObjectAsTask(action, state), preferLocal: true);
#elif NETSTANDARD2_0
ThreadPool.QueueUserWorkItem(_actionObjectAsTask, new ActionObjectAsTask(action, state));
#else
Task.Factory.StartNew(action, state);
#endif
}

#if NETCOREAPP2_1 || NETSTANDARD2_0
// Catches only the exception into a failed Task, so the fire-and-forget action
// can be queued directly to the ThreadPool without the extra overhead of as Task
private readonly static WaitCallback _actionAsTask = state =>
{
try
{
((Action)state)();
}
catch (Exception ex)
{
// Create faulted Task for the TaskScheulder to handle exception
// rather than letting it escape onto the ThreadPool and crashing the process
Task.FromException(ex);
}
};

private readonly static WaitCallback _actionObjectAsTask = state => ((ActionObjectAsTask)state).Run();

private sealed class ActionObjectAsTask
{
private Action<object> _action;
private object _state;

public ActionObjectAsTask(Action<object> action, object state)
{
_action = action;
_state = state;
}

// Catches only the exception into a failed Task, so the fire-and-forget action
// can be queued directly to the ThreadPool without the extra overhead of as Task
public void Run()
{
try
{
_action(_state);
}
catch (Exception ex)
{
// Create faulted Task for the TaskScheulder to handle exception
// rather than letting it escape onto the ThreadPool and crashing the process
Task.FromException(ex);
}
}
}
#endif
}
}