Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Shrink Task.Delay when used without Cancellation (2)#22234

Closed
benaadams wants to merge 2 commits into
dotnet:masterfrom
benaadams:TaskDelay
Closed

Shrink Task.Delay when used without Cancellation (2)#22234
benaadams wants to merge 2 commits into
dotnet:masterfrom
benaadams:TaskDelay

Conversation

@benaadams
Copy link
Copy Markdown
Member

@benaadams benaadams commented Jan 26, 2019

Follow up to #22233

Task.Delay is commonly used without cancellation:

Task Delay(int) - API Port 22.6%
Task Delay(TimeSpan) - API Port 9.8%

Task Delay(int, CancellationToken) - API Port 5.6%
Task Delay(TimeSpan, CancellationToken) - API Port 9.3%

This shaves 24 bytes of allocation off that common path.

Using @SergeyTeplyakov's ObjectLayoutInspector and local copies of the internal objects to make it easier to inspect.

ObjectLayoutInspector.TypeLayout.PrintLayout<DelayPromiseWithCancellation>(recursively: true);
ObjectLayoutInspector.TypeLayout.PrintLayout<DelayPromise>(recursively: true);
Type layout for 'DelayPromiseWithCancellation'
Size: 88 bytes. Paddings: 7 bytes (%7 of empty space)
|==============================================================|
| Object Header (8 bytes)                                      |
|--------------------------------------------------------------|
| Method Table Ptr (8 bytes)                                   |
|==============================================================|
|   0-7: Delegate m_action (8 bytes)                           |
|--------------------------------------------------------------|
|  8-15: Object m_stateObject (8 bytes)                        |
|--------------------------------------------------------------|
| 16-23: TaskScheduler m_taskScheduler (8 bytes)               |
|--------------------------------------------------------------|
| 24-31: Object m_continuationObject (8 bytes)                 |
|--------------------------------------------------------------|
| 32-39: ContingentProperties m_contingentProperties (8 bytes) |
|--------------------------------------------------------------|
| 40-43: Int32 m_taskId (4 bytes)                              |
|--------------------------------------------------------------|
| 44-47: Int32 m_stateFlags (4 bytes)                          |
|--------------------------------------------------------------|
|    48: VoidTaskResult m_result (1 byte)                      |
|--------------------------------------------------------------|
| 49-55: padding (7 bytes)                                     |
|--------------------------------------------------------------|
| 56-63: TimerQueueTimer Timer (8 bytes)                       |
|--------------------------------------------------------------|
| 64-71: CancellationToken Token (8 bytes)                     |
|--------------------------------------------------------------|
| 72-87: CancellationTokenRegistration Registration (16 bytes) |
| |=====================================|                      |
| |   0-7: CallbackNode _node (8 bytes) |                      |
| |-------------------------------------|                      |
| |  8-15: Int64 _id (8 bytes)          |                      |
| |=====================================|                      |
|==============================================================|

To

Type layout for 'DelayPromise'
Size: 64 bytes. Paddings: 7 bytes (%10 of empty space)
|==============================================================|
| Object Header (8 bytes)                                      |
|--------------------------------------------------------------|
| Method Table Ptr (8 bytes)                                   |
|==============================================================|
|   0-7: Delegate m_action (8 bytes)                           |
|--------------------------------------------------------------|
|  8-15: Object m_stateObject (8 bytes)                        |
|--------------------------------------------------------------|
| 16-23: TaskScheduler m_taskScheduler (8 bytes)               |
|--------------------------------------------------------------|
| 24-31: Object m_continuationObject (8 bytes)                 |
|--------------------------------------------------------------|
| 32-39: ContingentProperties m_contingentProperties (8 bytes) |
|--------------------------------------------------------------|
| 40-43: Int32 m_taskId (4 bytes)                              |
|--------------------------------------------------------------|
| 44-47: Int32 m_stateFlags (4 bytes)                          |
|--------------------------------------------------------------|
|    48: VoidTaskResult m_result (1 byte)                      |
|--------------------------------------------------------------|
| 49-55: padding (7 bytes)                                     |
|--------------------------------------------------------------|
| 56-63: TimerQueueTimer Timer (8 bytes)                       |
|==============================================================|

/cc @stephentoub

@stephentoub
Copy link
Copy Markdown
Member

Thanks. I was thinking of something slightly different. Away from my computer right now but will try it when I'm back.

@stephentoub
Copy link
Copy Markdown
Member

stephentoub commented Jan 27, 2019

@benaadams, I was thinking of something like this:
stephentoub@37378d5
Opinions? (This also addresses an existing race condition I noticed in the implementation.)

@benaadams
Copy link
Copy Markdown
Member Author

Yeah, that looks much better

@benaadams
Copy link
Copy Markdown
Member Author

Are you submitting PR with it?

@stephentoub
Copy link
Copy Markdown
Member

Are you submitting PR with it?

Feel free to copy or cherry-pick, since you already have two PRs open 😄

@benaadams
Copy link
Copy Markdown
Member Author

Done, went for first as my cherry-picking ability is still a bit primordial 😄

@benaadams benaadams deleted the TaskDelay branch January 27, 2019 16:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants