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#22233

Merged
stephentoub merged 1 commit into
dotnet:masterfrom
benaadams:non-cancel-taskdelay
Jan 28, 2019
Merged

Shrink Task.Delay when used without Cancellation#22233
stephentoub merged 1 commit into
dotnet:masterfrom
benaadams:non-cancel-taskdelay

Conversation

@benaadams
Copy link
Copy Markdown
Member

@benaadams benaadams commented Jan 26, 2019

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<DelayPromise>(recursively: true);
ObjectLayoutInspector.TypeLayout.PrintLayout<DelayPromiseWithoutCancellation>(recursively: true);
Type layout for 'DelayPromise'
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 'DelayPromiseWithoutCancellation'
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

@benaadams
Copy link
Copy Markdown
Member Author

@dotnet test OSX10.12 x64 Checked Innerloop Build and Test

Comment thread src/System.Private.CoreLib/shared/System/Threading/Tasks/Task.cs
Comment thread src/System.Private.CoreLib/shared/System/Threading/Tasks/Task.cs Outdated
@Wraith2
Copy link
Copy Markdown

Wraith2 commented Jan 26, 2019

That 7 bytes of padding bothers me. Is there a way to mask the voidresult into the state flags?

@stephentoub
Copy link
Copy Markdown
Member

Is there a way to mask the voidresult into the state flags?

No. That's just the generic TResult. The right answer there is to move the promise functionality down from the generic task into the non-generic one. It's on my to-do list to explore that.

@benaadams
Copy link
Copy Markdown
Member Author

Is there a way to mask the voidresult into the state flags?

No. That's just the generic TResult. The right answer there is to move the promise functionality down from the generic task into the non-generic one. It's on my to-do list to explore that.

Could move m_taskId into m_contingentProperties, then it would drop to 3 bytes ;)

Though it would go back to 4 bytes after making it non-generic (e.g. moving m_taskId would be a wash at that point)

@Wraith2
Copy link
Copy Markdown

Wraith2 commented Jan 27, 2019

I just hadn't realised it was T result, it isn't really wasted padding.

@benaadams benaadams force-pushed the non-cancel-taskdelay branch from 8174eef to 28584ce Compare January 27, 2019 16:09
@stephentoub
Copy link
Copy Markdown
Member

The right answer there is to move the promise functionality down from the generic task into the non-generic one. It's on my to-do list to explore that.

#22238

@benaadams benaadams force-pushed the non-cancel-taskdelay branch from 28584ce to 7e9a8cb Compare January 27, 2019 16:36
@benaadams
Copy link
Copy Markdown
Member Author

Fixed commits; change should be the same

@benaadams
Copy link
Copy Markdown
Member Author

Though I think I have them reversed O_o

@benaadams
Copy link
Copy Markdown
Member Author

@dotnet-bot test OSX10.12 x64 Checked Innerloop Build and Test

@jkotas
Copy link
Copy Markdown
Member

jkotas commented Jan 27, 2019

@benaadams Could you please resolve the conflict?

@stephentoub
Copy link
Copy Markdown
Member

Resolved.

@stephentoub stephentoub merged commit 8a29aa1 into dotnet:master Jan 28, 2019
Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/corefx that referenced this pull request Jan 28, 2019
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/corert that referenced this pull request Jan 28, 2019
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
jkotas pushed a commit to dotnet/corert that referenced this pull request Jan 28, 2019
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
@benaadams benaadams deleted the non-cancel-taskdelay branch January 28, 2019 03:23
jkotas pushed a commit to dotnet/corefx that referenced this pull request Jan 28, 2019
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/mono that referenced this pull request Jan 28, 2019
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
marek-safar pushed a commit to mono/mono that referenced this pull request Jan 28, 2019
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
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.

4 participants