Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove some low-hanging fruit allocation #821

Merged
merged 1 commit into from
Sep 17, 2024

Conversation

stephentoub
Copy link
Contributor

@KirillOsenkov asked me to look at why the .NET 8 build was slower than the .NET Framework build (answer: #819), and why so much time was being taken up in ReadByte (answer: #820, dotutils/streamutils#2). But while looking into it, I did an allocation profile and noticed some low-hanging fruit easily avoided, so this addresses those.

  • Avoid closures when calling TreeNode.Find{Last}Child. Added an overload that takes state, and made all call sites use static to help flag places state was being closed over.
  • Avoid closures in BuildEventArgs.Read. The error paths were closing over state used by the success path, resulting in allocation even on success.
  • Changed some methods that were returning interfaces to return the strongly-typed collection instead. Consumers that were iterating through them can now automatically avoid going through an enumerator.
  • Avoid params allocations for most call sites to FormatResourceStringIgnoreCodeAndKeyword by adding dedicated overloads for 1/2/3 args.
  • Avoid an int boxing allocation (on core) when concatening a string with an int.
  • Updated TextUtilities to use more modern APIs, e.g. MemoryExtensions.IndexOfAny, IndexOf, ReplaceLineEndings, etc. Some uses were (accidentally?) using Enumerable.Contains.

- Avoid closures when calling TreeNode.Find{Last}Child. Added an overload that takes state, and made all call sites use `static` to help flag places state was being closed over.
- Avoid closures in BuildEventArgs.Read. The error paths were closing over state used by the success path, resulting in allocation even on success.
- Changed some methods that were returning interfaces to return the strongly-typed collection instead. Consumers that were iterating through them can now automatically avoid going through an enumerator.
- Avoid params allocations for most call sites to FormatResourceStringIgnoreCodeAndKeyword by adding dedicated overloads for 1/2/3 args.
- Avoid an int boxing allocation (on core) when concatening a string with an int.
- Updated TextUtilities to use more modern APIs, e.g. MemoryExtensions.IndexOfAny, IndexOf, ReplaceLineEndings, etc. Some uses were (accidentally?) using Enumerable.Contains.
@KirillOsenkov KirillOsenkov merged commit 53fb2ee into KirillOsenkov:main Sep 17, 2024
1 check passed
@stephentoub stephentoub deleted the lowhangingalloc branch September 17, 2024 17:45
@KirillOsenkov
Copy link
Owner

KirillOsenkov commented Sep 17, 2024

Original:
image

This PR:
image

Allocations are down from 86.8 GB -> 84.4 GB! Very nice!

The time probably needs to be measured several times to be accurate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants