Suppress the execution context per operation #2974
Conversation
| @@ -17,9 +18,12 @@ public SocketAwaitableEventArgs WaitForDataAsync() | |||
| { | |||
| _awaitableEventArgs.SetBuffer(Array.Empty<byte>(), 0, 0); | |||
|
|
|||
| if (!_socket.ReceiveAsync(_awaitableEventArgs)) | |||
| using (ExecutionContext.SuppressFlow()) | |||
There was a problem hiding this comment.
Can the follow already be suppressed when it gets here? (in which case this call would throw)
There was a problem hiding this comment.
Unlikely but I can add the crazy code. Regardless, I’m sad this is per call instead of per construction
There was a problem hiding this comment.
The flow can't be suppressed here, this API is called from a single place in the code base.
Not sure what you mean? The regular tests? Yes they run as part of the PR and the execution context doesn't need to be captured here because it's already captured and restored by the calling code. |
|
@dotnet-bot test Windows Debug x64 Build |
|
What's the primary motivation for this: perf or less stack frames in debug/exception output? Am I correct in understanding you expect the perf to stay the same? |
Perf should be better or the same. It'll be absolutely better when we get dotnet/corefx#32582. |
|
Looks slightly faster on the PlatformPlaintext benchmark: Before
After
|
|
|
||
| protected AsyncFlowControl? SuppressExecutionContext() | ||
| { | ||
| return ExecutionContext.IsFlowSuppressed() ? (AsyncFlowControl?)null : ExecutionContext.SuppressFlow(); |
There was a problem hiding this comment.
You can using a null nullable? (Tested, yes you can)
That's nice and elegant :)
|
Running the performance numbers again to make sure they weren't all noise: Before
After
Might be a wash. |
|
Should skip out this bit |
Today we capture and restore the execution context per Socket operation. There's no need to do that because it already happens in the async state machine.
I need to measure the performance since we're making an additional method call per operation to save it on the other side, it might be a wash. Ideally, we would just use https://github.com/dotnet/corefx/issues/32582
BEFORE
AFTER
Look at that beautiful flattened frame 😄. This is the code being avoided https://github.com/dotnet/coreclr/blob/302630ed5a3730470e9ffeeebcd38c737c03963d/src/System.Private.CoreLib/shared/System/Threading/ExecutionContext.cs#L126-L202