IL: fix the per-reader string cache sizing - #20261
Open
auduchinok wants to merge 1 commit into
Open
Conversation
ILMetadataReader keeps two string tables per referenced assembly. Both were sized from something unrelated to how many strings are actually read, and the second one had nothing to cache. cacheStringHeap was sized stringsStreamSize / 50 + 1, i.e. from the length of the #Strings stream. Only a small fraction of a #Strings heap is ever read, so the table sat around 11% full: one nearly-empty table per reference. It is now sized to grow. memoizeString had a single caller, the ns + "." + name concatenation in readBlobHeapAsTypeName. Every caller of that function is already cached or one-shot per row (typeDefReader, seekReadTypeDefAsTypeRefUncached, seekReadTypeRefUncached, and the exported-type readers), so the concatenation happens about once per typedef, typeref or exported-type row, and the table could only pay when two different rows produced identical text: the same name under a different resolution scope, or a type forwarder. Measured within-assembly retained string duplication is 0.00 MB, so it collapsed nothing, while holding every namespaced type name alive for the reader's lifetime as both key and value. Removed. Retained memory after ParseAndCheckProject drops 1.5-10.1 MB per project (-1.4% to -6.4%) across ten projects, and total allocation drops 1-34 MB. The saving scales with the number of referenced assemblies rather than project size, since the cost was two tables per reader, so the smallest subject gains most in relative terms and the one with 489 references gains most in absolute terms. Analysis time is unchanged within measurement noise. Tables.memoize still has a caller in ilmorph.fs, so it stays. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
❗ Release notes requiredYou can open this PR in browser to add release notes: open in github.dev
Warning No PR link found in some release notes, please consider adding it.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ILMetadataReaderkeeps two string tables per referenced assembly. Both were sized from somethingunrelated to how many strings are actually read.
cacheStringHeapwas sizedstringsStreamSize / 50 + 1, i.e. from the length of the#Stringsstream.Only a small fraction of a
#Stringsheap is ever read, so the table sat around 11% full — onenearly-empty table per reference. It is now sized to grow.
memoizeStringinterned one computed string: thens + "." + nameconcatenation inreadBlobHeapAsTypeName. Every caller of that function is already cached or one-shot per row(
typeDefReader,seekReadTypeDefAsTypeRefUncached,seekReadTypeRefUncached, and the exported-typereaders), so the table could only pay when two different rows produce identical text. It went through
Tables.memoize, whose fixed 1000-entry capacity cost about 3.0 MB in buckets across a large referenceset while collapsing at most 0.24 MB of duplicate names — so the table is removed rather than resized.
Tables.memoizekeeps itsilmorph.fscaller.Retained memory after
ParseAndCheckProject, mean of 3 runs in fresh processes per project:The saving is two tables per reader, so it scales with the number of referenced assemblies rather than
project size: the smallest subject gains most in relative terms, and FSharp.Common, with 489 references,
most in absolute terms. Total allocation drops 1-34 MB per project. Analysis time is unchanged within
measurement noise.
Without the intern table, ~2,077 duplicate name strings survive per FSharp.Common analysis (~0.24 MB,
an upper bound). 98% are one structural pair: the same type def read once as
ILTypeDef.Nameand onceas
ILTypeRef.Name. That is the 0.24 MB the table used to collapse, and it is well below what the tableitself cost.