-
Notifications
You must be signed in to change notification settings - Fork 506
Recover some of the size lost with AsyncMethodBuilder/Task unification #6913
Conversation
When we unified the implementations of these across all of our runtimes, we lost all size optimizations people have been doing on the Project N side over the past six years. This restores a bit of the loss. For one sample app with lots of async usage, this removes 2.1 MB of generic instantiations. There is more we can do, but I can't spend time on that right now. These two things jumped out on me when I was looking at it back in December and were an easy fix I wanted to do for a while.
|
BTW: A few callbacks have been converted from explicit ones to lamdas in dotnet/coreclr#22172 . Hopefully, this cleanup is fine since it is in non-generic type. |
|
@jaredpar The Roslyn creates dummy singleton helper objects for "static" lambdas. I assume that it is a micro-optimization to avoid overhead of shuffle thunks. Do you happen to know the PR or piece of code that introduced this optimization? We have been wondering whether the Roslyn policy on when to produce these dummy static singletons should be revisited since it adds significant static overhead to frequently instantiated generics. |
| { | ||
| /// <summary>Delegate used to invoke on an ExecutionContext when passed an instance of this box type.</summary> | ||
| private static readonly ContextCallback s_callback = s => | ||
| private static readonly ContextCallback s_callback = ExecutionContextCallback; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tried to move the lambda to the body? This forces static ctor which looks like is not a common path here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This forces static ctor which looks like is not a common path here.
This callback is common path. Unless you've explicitly suppressed ExecutionContext flow with ExecutionContext.SuppressFlow(), this callback will be used every time the async state machine resumes after an await.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, before-field-init static ctors are executed lazily in CoreCLR so they pose no problem there. IIRC, it is different in Mono - before-field-init static ctors are eager in Mono that makes use of the before-field-init as a startup and static footprint optimization ineffective.
This change was done long before Roslyn went OSS. Hence you're unlikely to find it in our history anywhere.
@VSadov originally did this change hence he may have more feedback here. But my understannding is that the savings were pretty significant for Roslyn + VS when the change was made. |
|
Yes. this is a very old change by now. It was prior Roslyn v1.0
|
I didn't remember that the ability to cache generic lambdas was tied to that. We do rely on the generic lambda caching in a large number of places; those could be unwound, but it results in writing code in a fairly ugly / pre-lambda manner. |
|
I can imagine using different lowering strategy if we had There could be other places where codegen could be less generic and/or more compact (possible at the cost of some throughput) - anonymous types, async stack spilling.... |
|
@stephentoub generic lambda caching came naturally from the change since generic "static" lambdas are no longer backed by static delegates and there is a containing type to put the static cache field. It might be possible to untangle just generic lambda caching from the rest of the change, if needed. |
When we unified the implementations of these across all of our runtimes, we lost all size optimizations people have been doing on the Project N side over the past six years.
This restores a bit of the loss. For one sample app with lots of async usage, this removes 2.1 MB of generic instantiations.
There is more we can do, but I can't spend time on that right now. These two things jumped out on me when I was looking at it back in December and were an easy fix I wanted to do for a while.