Remove lazy initialization of Task.CompletedTask#8846
Remove lazy initialization of Task.CompletedTask#8846stephentoub merged 2 commits intodotnet:masterfrom
Conversation
The lazy-initialization code of Task.CompletedTask is causing it to be an "unprofitable inline". For workloads that make heavy use of Task-returning methods that complete synchronously, this is showing up as a measurable impact, e.g. in a simple benchmark reading and writing to the channels in the channels lib on corefxlabs, it's resulting in 2-5% overhead. However, as it's used by a bunch of core .NET types, e.g. a bunch of streams like FileStream and MemoryStream, it's rare that it won't be needed, and it's often needed very early in a program's execution. This commit just removes the lazy-initialization, removing a branch and allowing the getter to be inlined.
|
Based on #1157 leading to #4340, I'm not seeing a downside here. |
| @@ -1617,21 +1617,8 @@ internal TaskScheduler ExecutingTaskScheduler | |||
| /// </remarks> | |||
| public static TaskFactory Factory { get { return s_factory; } } | |||
There was a problem hiding this comment.
Nit - it may be nice to change this one to use the same syntax as CompletedTask to make the style locally consistent.
|
Interesting; always wondered why it showed up on traces - never thought to look though. |
|
Upstream workarounds TaskCache - though also deal with the missing api in NET451 |
|
@jkotas, shall I port this to corert, or is there any reason to keep it lazy there? (I ask because it looks like the TaskFactory property there was made lazy in the port to .NET Native / corert.) |
|
Yes, I think it should be ported to CoreRT. I do not think there is a good reason for CoreRT to differ for these two properties. The TaskFactory laziness was introduced as part of some AOT binary size micro-optimizations in early days of .NET Native. I do not think these micro-optimizations are good in this case. They trade small binary size gain in some cases for measurable throughput loss. |
Remove lazy initialization of Task.CompletedTask
Remove lazy initialization of Task.CompletedTask Commit migrated from dotnet/coreclr@c586863
The lazy-initialization code of Task.CompletedTask is causing it to be an "unprofitable inline". For workloads that make heavy use of Task-returning methods that complete synchronously, this is showing up as a measurable impact, e.g. in a simple benchmark reading and writing to the channels in the channels lib on corefxlabs, it's resulting in 2-5% overhead. However, as it's used by a bunch of core .NET types, e.g. a bunch of streams like FileStream and MemoryStream, it's rare that it won't be needed, and it's often needed very early in a program's execution. This commit just removes the lazy-initialization, removing a branch and allowing the getter to be inlined.
cc: @jkotas, @AndyAyersMS