Reuse the scope factory of the other flavour - #6114
Merged
Merged
Conversation
create() memoizes the services it resolves out of the container, but toFiberFactory()/toMutatingFactory() returned a freshly constructed factory every time, so those memos never survived a scope switching flavour — and scopes switch constantly. On a self-analysis of src/Type that was 192.8K factory constructions, each re-reading two services in the constructor and then resolving nine more on its first create(): 1.6M MemoizingContainer::getByType calls where a few thousand suffice. Pairing the two factories brings the constructions down to 791 and getByType to 94.7K. The pair is held one way strongly and the other way weakly. ScopeFactory makes one of these factories per analysed file, closing over that file's node callback, so a strong cycle between the two would keep every file's callback — and everything it captures — alive for the whole run: PHPStan disables the cycle collector, and nothing else would ever free them. Peak RSS of a src self-analysis measured 678MB before, 1061MB with a strong cycle, and 678MB with the weak back-reference. Interleaved A/B on a quiet machine, user CPU, result cache cleared, turbo extension enabled: -1.40% on src/Type (21.58s -> 21.28s, 6/8 pairs, paired t=3.0); peak RSS over the same pairs is unchanged (-0.35%, t=0.3). Analysis output is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016XhKccs7xeB1wRTAQEemc2
ondrejmirtes
force-pushed
the
scope-factory-twin
branch
from
July 27, 2026 19:45
24fec99 to
5b839e5
Compare
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
1 participant
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.
LazyInternalScopeFactory::create()memoizes the nine services it resolves out of the container with??=. Those memos never survived, becausetoFiberFactory()andtoMutatingFactory()returned a freshly constructed factory on every call — and scopes switch flavour constantly (MutatingScope::toFiberScope(),FiberScope::toMutatingScope()).Every switch therefore built a factory that re-read two services in its constructor and then resolved nine more on its first
create().Found while profiling with the turbo extension enabled (SPX,
SPX_FP_LIMIT=20000) for something unrelated — the container showing up with 1.6MgetByTypecalls is what gave it away.Effect on a self-analysis of
src/TypeLazyInternalScopeFactory::__constructMemoizingContainer::getByTypegetParameter/getService/getExtensionsCollectionBenchmark
Interleaved A/B, pair order alternated (ABBA), user CPU, result cache cleared before every run, turbo extension enabled, machine otherwise idle:
srcsrc/TypeCorrectness
The factory is immutable apart from those memo fields, so returning a shared instance where a new one used to be built is equivalent — the two flavours differ only in the
$fiberflag. The twin is paired both ways, so a scope round-tripping between flavours reuses the original rather than allocating a third factory.--error-format=rawoutput oversrcis unchanged, andtests/PHPStan/Analyser(2869 tests) plustests/PHPStan/Rules/Methods,Rules/Variables,NodeandType(3819 tests) pass.🤖 Generated with Claude Code
https://claude.ai/code/session_016XhKccs7xeB1wRTAQEemc2