-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Fast path ExecutionContext.Default for AsyncLocal #8227
Conversation
|
Can you share measurements? |
| { | ||
| ExecutionContext current = Thread.CurrentThread.ExecutionContext; | ||
| if (current == null) | ||
| ExecutionContext current = Thread.CurrentThread.ExecutionContext ?? ExecutionContext.Default; |
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 always going to make things slower.
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.
Seems like the only case it could help is accessing AsyncLocal.Value when no values have been set, in which case it'll add the branch but save an interface method call.
|
|
||
| if (newValue == null && newValues.Count == 0) | ||
| { | ||
| // Async locals cleared - set ExecutionContext back to Default to move back to fast-path |
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.
How likely is it that the async locals are ever cleared? It feels like a rare corner case.
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 would happen if all AsyncLocals in use were with T == reference types and all of them had their value set back to null (any T == value type in existence or any reference type not being set back to null would thwart this). I agree that does seem relatively rare.
| if (newValue == null && newValues.Count == 0) | ||
| { | ||
| // Async locals cleared - set ExecutionContext back to Default to move back to fast-path | ||
| Thread.CurrentThread.ExecutionContext = ExecutionContext.Default; |
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.
ExecutionContext contains more than just the async locals. Do you also need to compare the rest of the state?
|
@benaadams have you had a chance to progress on this one? |
|
It needs a deep dive; will close and come back when have something with results 😄 |
Move back to Default ExecutionContext when AsyncLocals are cleared
Early exit if setting value to null on DefaultContext
Return
nullfor get if context is default without checking./cc @stephentoub @jkotas