Use AsyncTaskCache in ValueTask.AsTask() #13907
Conversation
We can avoid task allocations for common values by using the same task cache that async methods do. This is important to avoid allocations in certain cases when switching from Task-returning to ValueTask-returning methods. If at some point we change Task.FromResult to use the same cache, this can be reverted.
|
LGTM.
Is there anything preventing us to do that? I mean would be any compat or behavior concern? or just not a priority to do so now? |
Technically there's a potential compat issue, but I'm not concerned about it. It's just the case where if you depend on object identity and we switch from always giving out a new object to potentially giving out a cached one, it could break such object identity comparisons. But we already decided for core we don't care about that (e.g. returning cached empty arrays), and honestly in this case I think it'd even be worth considering for desktop.
I would like to see it happen for 2.1. We just need to find the time. Actually making the change is trivial; we just need to make sure not to regress perf. The issue is that there are only a handful of tasks actually cached, e.g. true/false for bool, -1 to 8 for int, 0 values, etc., but Task.FromResult is used with lots of values, so we need to make sure the overhead of accessing the cache is worthwhile. |
Put a PR in for |
…task_caching Use AsyncTaskCache in ValueTask.AsTask() Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
…task_caching Use AsyncTaskCache in ValueTask.AsTask() Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
We can avoid task allocations for common values by using the same task cache that async methods do. This is important to avoid allocations in certain cases when switching from Task-returning to ValueTask-returning methods. If at some point we change Task.FromResult to use the same cache, this can be reverted.
cc: @kouvel, @tarekgh