IOThreadScheduler using IO Threads on Windows #4282
Conversation
| @@ -295,6 +296,16 @@ public static Action<T1> ThunkCallback<T1>(Action<T1> callback) | |||
| return (new ActionThunk<T1>(callback)).ThunkFrame; | |||
| } | |||
|
|
|||
| #pragma warning disable CS3002 // Return type is not CLS-compliant | |||
| #pragma warning disable CS3001 // Argument type is not CLS-compliant | |||
| public static IOCompletionCallback ThunkCallback(IOCompletionCallback callback) | |||
| } | ||
| catch (Exception exception) | ||
| { | ||
| if (!Fx.HandleAtThreadBase(exception)) |
There was a problem hiding this comment.
Could use exception filters of you wanted
| } | ||
|
|
||
| bool queued = false; | ||
| while (!queued) | ||
| { | ||
| try { } | ||
| try | ||
| { } | ||
| finally |
There was a problem hiding this comment.
Is this code run on platforms other than .NET Core? This try/finally pattern isn't applicable to core, which lacks thread aborts.
| // by the GC anyway. | ||
| private unsafe class ScheduledOverlapped | ||
| { | ||
| private static bool s_isWindows = RuntimeInformation.IsOSPlatform(OSPlatform.Windows); |
There was a problem hiding this comment.
readonly. It'll be optimized like a const at tier 1.
| if (callback != null) | ||
| { | ||
| callback(state); | ||
| } |
There was a problem hiding this comment.
FWIW, you can now do things like:
callback?.Invoke(state);
| Fx.Assert(iots != null, "Post called with a null scheduler."); | ||
|
|
||
| _scheduler = iots; | ||
| _postDelegate(); |
There was a problem hiding this comment.
Why go through a delegate here?
| } | ||
| else | ||
| { | ||
| _postDelegate = PostNewThread; |
There was a problem hiding this comment.
I think we should experiment with this being a QieueUserWorkItem, but that can be a discussion for another day :-)
There was a problem hiding this comment.
I feel like it can’t really hurt but we can leave it
| private void PostNewThread() | ||
| { | ||
| var thread = new Thread(new ThreadStart(Callback)); | ||
| thread.Start(); |
There was a problem hiding this comment.
This should set IsBackground to true before starting the thread.
| @@ -12,7 +12,7 @@ public class ServiceModelSynchronizationContext : SynchronizationContext | |||
|
|
|||
| public override void Post(SendOrPostCallback d, object state) | |||
| { | |||
| IOThreadScheduler.ScheduleCallbackNoFlow(d, state); | |||
| IOThreadScheduler.ScheduleCallbackNoFlow((s) => d(s), state); | |||
There was a problem hiding this comment.
This is going to allocate a closure and a delegate on every invocation. Is it called low-frequency enough that the overhead doesn't matter?
| } | ||
| else | ||
| { | ||
| await Task.Factory.FromAsync(other.BeginClose(timeout, callback: null, state: null), other.EndClose); | ||
| return Task.Factory.FromAsync(other.BeginClose, other.EndClose, timeout, null); |
There was a problem hiding this comment.
I thought I remember you saying that everything was implemented via tasks and then APM was wrapped around those. This is the inverse. Is this just a rarity?
Co-authored-by: Matt Connew <[email protected]>
Scheduling a delegate via overlapped to the IO thread pool wasn't available when this project started. It is now available on Windows so bringing back this functionality to prevent extra thread creation.