Reduce allocations observed in local trace opening Roslyn.sln #68076
Conversation
sharwell
commented
May 3, 2023
•
sharwell
commented
- First commit is from Update roslyn-integration-corehost default queue #68074 and is not part of this pull request
- Remaining commits should reviewed commit-by-commit
This CI definition should have the same default queue as roslyn-integration-CI.
Addresses 200MB of Large Object Heap allocations (40% of total LOH allocations) observed in a local trace loading Roslyn.sln.
Resolves 150MB allocations observed in a local 50 second trace opening Roslyn.sln.
Avoids 116MB allocations in a local 50 second performance trace opening Roslyn.sln.
Fixes 6.8MB allocations observed in a 50 second trace opening Roslyn.sln.
dotnet-issue-labeler
bot
added
Area-Compilers
untriaged
| return IsTypeLessVisibleThan(type1, arg.sym, ref localUseSiteInfo); | ||
| }, | ||
| (sym, localUseSiteInfoPtr), | ||
| canDigThroughNullable: true); // System.Nullable is public |
There was a problem hiding this comment.
/cc @stephentoub
src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/MethodTypeInference.cs
Outdated
Show resolved
Hide resolved
| { | ||
| CompoundUseSiteInfo<AssemblySymbol> localUseSiteInfo = useSiteInfo; | ||
| var result = type.VisitType((type1, symbol, unused) => IsTypeLessVisibleThan(type1, symbol, ref localUseSiteInfo), sym, | ||
| canDigThroughNullable: true); // System.Nullable is public | ||
| var localUseSiteInfoPtr = (nint)Unsafe.AsPointer(ref localUseSiteInfo); |
There was a problem hiding this comment.
It saves 150MB allocations over 50 seconds (also mentioned in the commit message).
There was a problem hiding this comment.
It saves 150MB allocations over 50 seconds (also mentioned in the commit message).
Sorry, I am having trouble translating this into user observable terms.
There was a problem hiding this comment.
I noticed the IDE going slow opening Roslyn.sln, so I took a performance trace. Much of the CPU usage was in GC. When this was combined with high overall allocation rates, it suggested that the best course of action would be take a pass at reducing allocations that occurred during this trace. This was one of the allocations that was more easily addressed, in terms of it being a fairly localized change.
There was a problem hiding this comment.
This was one of the allocations that was more easily addressed, in terms of it being a fairly localized change.
Has this change, on its own, made any noticeable impact on the scenario? And I am not talking about number of allocations in the trace,
There was a problem hiding this comment.
I have no reason to believe so, even though the evidence suggests this was a small contributing factor to the much larger picture. I was only able to address 1% of the scenario allocations in the time box. However, over time we would hope that the incremental improvements would accumulate to the point of being observable.
| { | ||
| CompoundUseSiteInfo<AssemblySymbol> localUseSiteInfo = useSiteInfo; | ||
| var result = type.VisitType((type1, symbol, unused) => IsTypeLessVisibleThan(type1, symbol, ref localUseSiteInfo), sym, | ||
| canDigThroughNullable: true); // System.Nullable is public | ||
| var localUseSiteInfoPtr = (nint)Unsafe.AsPointer(ref localUseSiteInfo); |
There was a problem hiding this comment.
The ref argument isn't guaranteed to be on the stack, so eliminating the local adds new gymnastics of a fixed statement (which ends up declaring a pointer to a non-blittable type and gives CS8500).
