Skip to content

Defer unused CoreLib cache initialization - #132576

Open
tannergooding wants to merge 4 commits into
dotnet:mainfrom
tannergooding:tannergooding-split-compareinfo-icu-cache
Open

Defer unused CoreLib cache initialization#132576
tannergooding wants to merge 4 commits into
dotnet:mainfrom
tannergooding:tannergooding-split-compareinfo-icu-cache

Conversation

@tannergooding

Copy link
Copy Markdown
Member

Moves the ICU-only CompareInfo search values and the EncodingTable name cache behind nested static holders. This preserves one-time initialization when those paths are used while allowing NativeAOT to trim them from applications that only need unrelated static state.

A locally built Release NativeAOT win-x64 Hello World with InvariantGlobalization=true measured:

BuildExecutableDelta
main993,280 bytes
CompareInfo holder971,264 bytes-22,016 bytes
Both holders968,704 bytes-24,576 bytes

The cumulative raw section changes are .text -12,800 bytes, .rdata -9,728 bytes, .data -1,024 bytes, .pdata -512 bytes, and .reloc -512 bytes. The final dependency graph no longer contains SearchValues.Create, EncodingTable::.cctor, or the ConcurrentDictionary<string, int> name-cache construction.

Note

This pull request was prepared with GitHub Copilot.

tannergoodingand others added 2 commits August 20, 2026 10:59
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI lite review requested due to automatic review settings August 20, 2026 18:11
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-globalization
See info in area-owners.md if you want to be subscribed.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR defers initialization of two rarely-needed CoreLib caches by moving them behind nested static holder types, allowing NativeAOT trimming to omit the cache initialization (and related dependencies) when those paths aren’t used.

Changes:

  • Move the EncodingTable name→codepage ConcurrentDictionary into a nested static holder to avoid initializing it unless GetCodePageFromName is called.
  • Move the ICU-only CompareInfoSearchValues<char> initialization into a nested static holder to avoid creating it unless the ASCII fast-path is exercised.
Show a summary per file
FileDescription
src/libraries/System.Private.CoreLib/src/System/Text/EncodingTable.csDefers ConcurrentDictionary<string,int> construction until GetCodePageFromName is invoked.
src/libraries/System.Private.CoreLib/src/System/Globalization/CompareInfo.Icu.csDefers ICU ASCII SearchValues<char> creation until the ordinal helper methods need it.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

@tannergooding

Copy link
Copy Markdown
MemberAuthor

A sibling to #132550. Just did some local analysis for obvious static constructor roots that looked unnecessary

-- CC. @MichalStrehovsky, @jkotas, @agocke this is a trivial improvement that's likely worth taking for .NET 11/12, but it also feels like a more general issue and is a really common pattern in how we and the broader ecosystem write code. This is likely worth explicitly handling in the linker, either via some explicit trimming support or a way (maybe even an attribute) to indicate we support it being "outlined" by the tooling into such helper shapes.

We shouldn't have to explicitly think that every unique static readonly field may require its own nested holder class. At worst this should be some analyzer that flags it for NAOT enabled libraries with a fixer if we believe handling it in the linker is not feasible.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 20, 2026 18:49

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 20, 2026 19:09

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@tannergooding

Copy link
Copy Markdown
MemberAuthor

Updated everything and savings measured locally are still the same.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@tannergooding@jkotas