Rewrite AnalyzeFileReference.GetSupportedLanguages without LINQ #49337
Conversation
I did an allocation profile of csc.dll for building a simple "hello world" with the configuration `dotnet build` employs on a simple `dotnet new console` app. There are ~194K allocations / ~19MB. And ~30K / ~1MB of those are occurring unnecessarily because of use of LINQ in one function. This just rewrites that function to not use LINQ.
|
The reference scenarios I've been monitoring produce roughly 200-250GiB allocations for compilation with analyzer scenarios. I'm not sure the 0.0004% improvement is worth the additional complexity here. I typically expect to see 1GiB for moderate complexity improvements and at least 100MiB for improvements that don't also improve readability. |
We don't care about simple apps? |
For reference, the work was right-click, convert to foreach, and fix up the select many at the end. |
Just to make sure it's clear for future readers, yes, we do believe simple applications are important. My comment was more related to the fact that not all measurable issues are observable, and it's my experience that this scenario is unlikely to be one of the observable ones. I have to be careful when looking at problems of the scale facing this component. If we were to set/keep the performance bar at the level used for this pull request, there would be almost limitless pull requests touching this very complex subsystem, and eventually one of them would break functionality when we miss something during review. The bar mentioned above has proven to be reasonable in the sense that meaningful progress across a range of scenarios is not blocked, while at the same time there have been very few unexpected changes in behavior. |
This eliminated 15% of all allocations when compiling a simple program, and did so by modifying a single function. If there are more possible PRs like that, we should absolutely take them. |
* upstream/master: (148 commits) Remove unnecessary ArrayBuilder in MakeCallWithNoExplicitArgument (dotnet#49377) Revert "Skip binary for determinism checking" Use deterministic metadata names for emitted anonymous type fields. (dotnet#49370) Reuse LSP solutions if they don't need re-forking (dotnet#49305) Small nullability fixes (dotnet#49279) Create default arguments during binding (dotnet#49186) Clean nits Backport dotnet#48420 to release/dev16.8 (dotnet#49357) Rewrite AnalyzeFileReference.GetSupportedLanguages without LINQ (dotnet#49337) Use .Any extension of ImmutableArray(Of Symbol) (dotnet#48980) fix 'remove unnecessary cast' when doing bitwise ops on unsigned values. Harden, document, cross-link XunitDisposeHook Simplify x86 hook Fix split comment exporting for VB. Code style fix Overwrite xunit's app domain handling to not call AppDomain.Unload Revert pragma suppression Remove blank line Revert file Move block structure code back to Features layer ...
I did an allocation profile of csc.dll for building a simple "hello world" with the configuration
dotnet buildemploys on a simpledotnet new consoleapp. There are ~194K allocations / ~19MB. And ~30K / ~1MB of those are occurring unnecessarily because of use of LINQ in one function. This just rewrites that function to not use LINQ.Before:
After:
Contributes to dotnet/msbuild#5876