-
Notifications
You must be signed in to change notification settings - Fork 347
Scheduler improvements #2026
Scheduler improvements #2026
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,8 +5,11 @@ namespace System.Threading | |
{ | ||
public abstract class Scheduler | ||
{ | ||
public static Scheduler TaskRun { get; } = new TaskRunScheduler(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why this change? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because you said so here #2026 (review) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should have been more specific, why the syntax change. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Storing it in a Scheduler.ThreadPool.Schedule((() => {}) So it remains a virtual call. Storing it in an exact type field Won't directly help pipelines as the |
||
public static Scheduler Inline { get; } = new InlineScheduler(); | ||
private static ThreadPoolScheduler _threadPoolScheduler = new ThreadPoolScheduler(); | ||
private static InlineScheduler _inlineScheduler = new InlineScheduler(); | ||
|
||
public static Scheduler ThreadPool => _threadPoolScheduler; | ||
public static Scheduler Inline => _inlineScheduler; | ||
|
||
public abstract void Schedule(Action action); | ||
public abstract void Schedule(Action<object> action, object state); | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
// See the LICENSE file in the project root for more information. | ||
|
||
using System.Threading.Tasks; | ||
|
||
namespace System.Threading | ||
{ | ||
internal sealed class ThreadPoolScheduler : Scheduler | ||
{ | ||
public override void Schedule(Action action) | ||
{ | ||
#if NETCOREAPP2_1 | ||
// Queue to low contention local ThreadPool queue; rather than global queue as per Task | ||
Threading.ThreadPool.QueueUserWorkItem(_actionWaitCallback, action, preferLocal: true); | ||
#elif NETSTANDARD2_0 | ||
Threading.ThreadPool.QueueUserWorkItem(_actionWaitCallback, 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 | ||
Threading.ThreadPool.QueueUserWorkItem(_actionObjectWaitCallback, new ActionObjectAsWaitCallback(action, state), preferLocal: true); | ||
#elif NETSTANDARD2_0 | ||
Threading.ThreadPool.QueueUserWorkItem(_actionObjectWaitCallback, new ActionObjectAsWaitCallback(action, state)); | ||
#else | ||
Task.Factory.StartNew(action, state); | ||
#endif | ||
} | ||
|
||
#if NETCOREAPP2_1 || NETSTANDARD2_0 | ||
private readonly static WaitCallback _actionWaitCallback = state => ((Action)state)(); | ||
|
||
private readonly static WaitCallback _actionObjectWaitCallback = state => ((ActionObjectAsWaitCallback)state).Run(); | ||
|
||
private sealed class ActionObjectAsWaitCallback | ||
{ | ||
private Action<object> _action; | ||
private object _state; | ||
|
||
public ActionObjectAsWaitCallback(Action<object> action, object state) | ||
{ | ||
_action = action; | ||
_state = state; | ||
} | ||
|
||
public void Run() => _action(_state); | ||
} | ||
#endif | ||
} | ||
} |
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 is probably the most unfortunate part of this change but meh..
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.
Meh indeed. It's in implementation details with zero bearings on the user's experience.
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.
In this case yes.