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

Remove several uses of VoidTaskResult #22238

Merged
jkotas merged 2 commits into
dotnet:masterfrom
stephentoub:voidtaskresult
Jan 27, 2019
Merged

Remove several uses of VoidTaskResult #22238
jkotas merged 2 commits into
dotnet:masterfrom
stephentoub:voidtaskresult

Conversation

@stephentoub
Copy link
Copy Markdown
Member

Currently TrySetResult/Canceled/Exception live on Task<T>. There's no reason TrySetCanceled and TrySetException need to live there, as they only access state from the base Task, and so can be moved down. TrySetResult needs the TResult, however in a variety of cases Task<T> is used with a VoidTaskResult, and for such cases we can just have a parameterless TrySetResult() on the base class as well, which can be used any time there is no TResult or when default(TResult) is the desired result. This lets us switch several cases where we were using Task<VoidTaskResult> to just be Task, which saves 8 bytes on the task instance on 64-bit. It also avoids an Interlocked.Exchange as part of the TrySetResult call.

This primarily affects Task.Delay and the non-generic variants of Task.WhenAll, ValueTask.AsTask(), Task.FromCanceled, and Task.FromException.

cc: @kouvel, @tarekgh, @benaadams, @jkotas

Currently TrySetResult/Canceled/Exception live on `Task<T>`.  There's no reason `TrySetCanceled` and `TrySetException` need to live there, as they only access state from the base `Task`, and so can be moved down.  `TrySetResult` needs the `TResult`, however in a variety of cases `Task<T>` is used with a `VoidTaskResult`, and for such cases we can just have a parameterless `TrySetResult()` on the base class as well, which can be used any time there is no `TResult` or when `default(TResult)` is the desired result.  This lets us switch several cases where we were using `Task<VoidTaskResult>` to just be `Task`, which saves 8 bytes on the task instance on 64-bit.  It also avoids an Interlocked.Exchange as part of the TrySetResult call.

This primarily affects Task.Delay and the non-generic variants of Task.WhenAll, ValueTask.AsTask(), Task.FromCanceled, and Task.FromException.
Copy link
Copy Markdown
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

Copy link
Copy Markdown
Member

@benaadams benaadams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice cleanup!

@stephentoub
Copy link
Copy Markdown
Member Author

@dotnet-bot test CentOS7.1 x64 Debug Innerloop Build please

@stephentoub
Copy link
Copy Markdown
Member Author

@dotnet-bot test Ubuntu x64 Checked Innerloop Build and Test please

@jkotas
Copy link
Copy Markdown
Member

jkotas commented Jan 27, 2019

Ubuntu is known failure (https://github.com/dotnet/coreclr/issues/19890). Passed in DevOps CI.

@jkotas jkotas merged commit 5d5d680 into dotnet:master Jan 27, 2019
Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/corefx that referenced this pull request Jan 27, 2019
* Remove defunct netstandard code from ValueTask

* Remove several uses of VoidTaskResult

Currently TrySetResult/Canceled/Exception live on `Task<T>`.  There's no reason `TrySetCanceled` and `TrySetException` need to live there, as they only access state from the base `Task`, and so can be moved down.  `TrySetResult` needs the `TResult`, however in a variety of cases `Task<T>` is used with a `VoidTaskResult`, and for such cases we can just have a parameterless `TrySetResult()` on the base class as well, which can be used any time there is no `TResult` or when `default(TResult)` is the desired result.  This lets us switch several cases where we were using `Task<VoidTaskResult>` to just be `Task`, which saves 8 bytes on the task instance on 64-bit.  It also avoids an Interlocked.Exchange as part of the TrySetResult call.

This primarily affects Task.Delay and the non-generic variants of Task.WhenAll, ValueTask.AsTask(), Task.FromCanceled, and Task.FromException.

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 27, 2019
* Remove defunct netstandard code from ValueTask

* Remove several uses of VoidTaskResult

Currently TrySetResult/Canceled/Exception live on `Task<T>`.  There's no reason `TrySetCanceled` and `TrySetException` need to live there, as they only access state from the base `Task`, and so can be moved down.  `TrySetResult` needs the `TResult`, however in a variety of cases `Task<T>` is used with a `VoidTaskResult`, and for such cases we can just have a parameterless `TrySetResult()` on the base class as well, which can be used any time there is no `TResult` or when `default(TResult)` is the desired result.  This lets us switch several cases where we were using `Task<VoidTaskResult>` to just be `Task`, which saves 8 bytes on the task instance on 64-bit.  It also avoids an Interlocked.Exchange as part of the TrySetResult call.

This primarily affects Task.Delay and the non-generic variants of Task.WhenAll, ValueTask.AsTask(), Task.FromCanceled, and Task.FromException.

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 27, 2019
* Remove defunct netstandard code from ValueTask

* Remove several uses of VoidTaskResult

Currently TrySetResult/Canceled/Exception live on `Task<T>`.  There's no reason `TrySetCanceled` and `TrySetException` need to live there, as they only access state from the base `Task`, and so can be moved down.  `TrySetResult` needs the `TResult`, however in a variety of cases `Task<T>` is used with a `VoidTaskResult`, and for such cases we can just have a parameterless `TrySetResult()` on the base class as well, which can be used any time there is no `TResult` or when `default(TResult)` is the desired result.  This lets us switch several cases where we were using `Task<VoidTaskResult>` to just be `Task`, which saves 8 bytes on the task instance on 64-bit.  It also avoids an Interlocked.Exchange as part of the TrySetResult call.

This primarily affects Task.Delay and the non-generic variants of Task.WhenAll, ValueTask.AsTask(), Task.FromCanceled, and Task.FromException.

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
stephentoub added a commit to dotnet/corefx that referenced this pull request Jan 27, 2019
* Remove defunct netstandard code from ValueTask

* Remove several uses of VoidTaskResult

Currently TrySetResult/Canceled/Exception live on `Task<T>`.  There's no reason `TrySetCanceled` and `TrySetException` need to live there, as they only access state from the base `Task`, and so can be moved down.  `TrySetResult` needs the `TResult`, however in a variety of cases `Task<T>` is used with a `VoidTaskResult`, and for such cases we can just have a parameterless `TrySetResult()` on the base class as well, which can be used any time there is no `TResult` or when `default(TResult)` is the desired result.  This lets us switch several cases where we were using `Task<VoidTaskResult>` to just be `Task`, which saves 8 bytes on the task instance on 64-bit.  It also avoids an Interlocked.Exchange as part of the TrySetResult call.

This primarily affects Task.Delay and the non-generic variants of Task.WhenAll, ValueTask.AsTask(), Task.FromCanceled, and Task.FromException.

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
jkotas pushed a commit to dotnet/corert that referenced this pull request Jan 27, 2019
* Remove defunct netstandard code from ValueTask

* Remove several uses of VoidTaskResult

Currently TrySetResult/Canceled/Exception live on `Task<T>`.  There's no reason `TrySetCanceled` and `TrySetException` need to live there, as they only access state from the base `Task`, and so can be moved down.  `TrySetResult` needs the `TResult`, however in a variety of cases `Task<T>` is used with a `VoidTaskResult`, and for such cases we can just have a parameterless `TrySetResult()` on the base class as well, which can be used any time there is no `TResult` or when `default(TResult)` is the desired result.  This lets us switch several cases where we were using `Task<VoidTaskResult>` to just be `Task`, which saves 8 bytes on the task instance on 64-bit.  It also avoids an Interlocked.Exchange as part of the TrySetResult call.

This primarily affects Task.Delay and the non-generic variants of Task.WhenAll, ValueTask.AsTask(), Task.FromCanceled, and Task.FromException.

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
marek-safar pushed a commit to mono/mono that referenced this pull request Jan 27, 2019
* Remove defunct netstandard code from ValueTask

* Remove several uses of VoidTaskResult

Currently TrySetResult/Canceled/Exception live on `Task<T>`.  There's no reason `TrySetCanceled` and `TrySetException` need to live there, as they only access state from the base `Task`, and so can be moved down.  `TrySetResult` needs the `TResult`, however in a variety of cases `Task<T>` is used with a `VoidTaskResult`, and for such cases we can just have a parameterless `TrySetResult()` on the base class as well, which can be used any time there is no `TResult` or when `default(TResult)` is the desired result.  This lets us switch several cases where we were using `Task<VoidTaskResult>` to just be `Task`, which saves 8 bytes on the task instance on 64-bit.  It also avoids an Interlocked.Exchange as part of the TrySetResult call.

This primarily affects Task.Delay and the non-generic variants of Task.WhenAll, ValueTask.AsTask(), Task.FromCanceled, and Task.FromException.

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
@stephentoub stephentoub deleted the voidtaskresult branch March 21, 2019 18:38
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
* Remove defunct netstandard code from ValueTask

* Remove several uses of VoidTaskResult

Currently TrySetResult/Canceled/Exception live on `Task<T>`.  There's no reason `TrySetCanceled` and `TrySetException` need to live there, as they only access state from the base `Task`, and so can be moved down.  `TrySetResult` needs the `TResult`, however in a variety of cases `Task<T>` is used with a `VoidTaskResult`, and for such cases we can just have a parameterless `TrySetResult()` on the base class as well, which can be used any time there is no `TResult` or when `default(TResult)` is the desired result.  This lets us switch several cases where we were using `Task<VoidTaskResult>` to just be `Task`, which saves 8 bytes on the task instance on 64-bit.  It also avoids an Interlocked.Exchange as part of the TrySetResult call.

This primarily affects Task.Delay and the non-generic variants of Task.WhenAll, ValueTask.AsTask(), Task.FromCanceled, and Task.FromException.


Commit migrated from dotnet/coreclr@5d5d680
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.

3 participants