instantiate conditional types without a combined mapper for the cache lookup - #64601
Closed
Max Schwenk (maschwenk) wants to merge 1 commit into
Closed
Max Schwenk (maschwenk) wants to merge 1 commit into
Max Schwenk (maschwenk) wants to merge 1 commit into
Conversation
… lookup instantiateTypeWorker built combineTypeMappers(t.mapper, m) for every conditional type it instantiated, but getConditionalTypeInstantiation only uses that mapper to compute the type arguments for the cache key, and builds a new mapper from them on a miss. getConditionalTypeInstantiationEx maps the outer type parameters the way CompositeTypeMapper.Map does instead, without allocating the composite. (Not mapTypeWithCompositeMapper: it goes through getMappedType, which first replaces a distributed type parameter with its constraint.) 38k-file program (37,943 files), median of 3, single threaded / 4 checkers: allocations 148.86M -> 144.51M (-4.35M, -2.9%) / 246.89M -> 238.12M (-8.8M, -3.6%) on main, 148.42M -> 144.08M / 247.09M -> 238.41M on top of the lazy member PRs and their follow-ups. Heap after check, symbols, types and instantiations unchanged: the composites were garbage right away, so this saves allocations and GC work, not retained memory. Prototype in a Rust port of the checker (4 checkers there assign files by directory locality): 4,340,555 / 7,677,745 composite mappers avoided on top of the lazy member PRs; Go's malloc count there goes down by 4,340,793 single threaded. There the mappers are arena-allocated, so -0.10 / -0.17 GB retained. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Author
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.
instantiateTypeWorkerbuildscombineTypeMappers(t.mapper, m)for every conditional type it instantiates.getConditionalTypeInstantiationonly uses that mapper to map the outer type parameters to the type arguments for the cache key, and on a miss it buildsnewTypeMapper(outerTypeParameters, typeArguments)from those. so the composite mapper is garbage right after the lookup either waythis adds
getConditionalTypeInstantiationEx(t, m1, mapper, ...), which maps the type parameters the wayCompositeTypeMapper.Mapdoes (m1first, then instantiate the result withmapperifm1changed it, elsemapperalone).getConditionalTypeInstantiationcalls it withm1 == nil, so the other callers don't change. (notmapTypeWithCompositeMapper: that goes throughgetMappedType, which first replaces a distributed type parameter with its constraint, andCompositeTypeMapper.Mapdoesn't)on our 38k-file program (37,943 files, 0 errors), median of 3, one process at a time:
allocs is
Memory allocsfrom--extendedDiagnostics. the composite mappers are garbage right away, so this saves allocations and GC work, not retained memory: heap after check, symbols, types and instantiations don't change. an instrumented port of the checker counts 4,340,555 composite mappers avoided single threaded on that stack, and go's malloc count there goes down by 4,340,793. check time is within noisesame diagnostics on every run.
go test ./...passes (except the macOS fsevents tests ininternal/fswatch, which time out here under load and pass on a rerun), the testrunner also withTS_TEST_PROGRAM_SINGLE_THREADED=false, lint and format are clean. no new test, the change has no observable effect besides allocationsused claude code to help write this, ive reviewed it