Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove another reference field from async state machines #83737

Merged
merged 1 commit into from Mar 22, 2023

Conversation

stephentoub
Copy link
Member

The async state machine Task-derived type currently adds three fields:

  • The StateMachine
  • An Action field for caching any delegate created to MoveNext
  • The ExecutionContext to flow to the next MoveNext invocation

The other pending PR gets rid of the Action field by using the unused Action field from the base Task for that purpose.

This PR gets rid of the ExecutionContext field by using the unused state object field from the base Task for that purpose. The field is exposed via the public AsyncState property, so this also uses a bit from the state flags field to prevent this state object from being returned from that property.

The combination of removing those two fields shaves 16 bytes off of every async Task state machine box on 64-bit. The only remaining field added by the state machine type is for the state machine itself, which is required.

@ghost
Copy link

ghost commented Mar 21, 2023

Tagging subscribers to this area: @dotnet/area-system-threading-tasks
See info in area-owners.md if you want to be subscribed.

Issue Details

The async state machine Task-derived type currently adds three fields:

  • The StateMachine
  • An Action field for caching any delegate created to MoveNext
  • The ExecutionContext to flow to the next MoveNext invocation

The other pending PR gets rid of the Action field by using the unused Action field from the base Task for that purpose.

This PR gets rid of the ExecutionContext field by using the unused state object field from the base Task for that purpose. The field is exposed via the public AsyncState property, so this also uses a bit from the state flags field to prevent this state object from being returned from that property.

The combination of removing those two fields shaves 16 bytes off of every async Task state machine box on 64-bit. The only remaining field added by the state machine type is for the state machine itself, which is required.

Author: stephentoub
Assignees: -
Labels:

area-System.Threading.Tasks

Milestone: 8.0.0

The async state machine Task-derived type currently adds three fields:
- The StateMachine
- An Action field for caching any delegate created to MoveNext
- The ExecutionContext to flow to the next MoveNext invocation

The other pending PR gets rid of the Action field by using the unused Action field from the base Task for that purpose.

This PR gets rid of the ExecutionContext field by using the unused state object field from the base Task for that purpose.  The field is exposed via the public AsyncState property, so this also uses a bit from the state flags field to prevent this state object from being returned from that property.

The combination of removing those two fields shaves 16 bytes off of every `async Task` state machine box on 64-bit.  The only remaining field added by the state machine type is for the state machine itself, which is required.
@mhmd-azeez
Copy link

The only remaining field added by the state machine type is for the state machine itself, which is required.

Looking forward to your next PR removing that field too

@davidfowl
Copy link
Member

I spent a little bit of time trying to understand if it made sense to hoist the ExecutionContext field to Task and use that instead, but then I realized it was on contingent properties. It does feel a tad bit icky since that field isn't being used here but the comments are good enough to understand why.

@stephentoub
Copy link
Member Author

I spent a little bit of time trying to understand if it made sense to hoist the ExecutionContext field to Task and use that instead, but then I realized it was on contingent properties. It does feel a tad bit icky since that field isn't being used here but the comments are good enough to understand why.

Right. Moving it back from contingent properties would end up penalizing TCS tasks (and others like them, e.g. Task.Delay, Task.WhenAll, etc.), as well as delegate-backed ones (e.g. Task.Run, Task.ContinueWith, etc.) when in the default ExecutionContext. I decided that all around this was the better tradeoff, since it doesn't make anything measurably more expensive and it makes async Task measurably faster/smaller. And it's all implementation detail, so we can change it again in the future should a better option present itself.

@stephentoub stephentoub merged commit e50b41e into dotnet:main Mar 22, 2023
167 checks passed
@stephentoub stephentoub deleted the asmec branch March 22, 2023 22:50
@dotnet dotnet locked as resolved and limited conversation to collaborators Apr 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants