Keep empty constant arrays out of the oversized merge in TypeCombinator::optimizeConstantArrays() - #6560
Merged
ondrejmirtes merged 2 commits intoSep 23, 2026
Conversation
…tor::optimizeConstantArrays()`
- optimizeConstantArrays() generalizes every non-empty constant array of an
oversized union to `non-empty-array<K, V>&oversized-array` and, when every
member got generalized, merges them into a single oversized array. An
`array{}` member is left alone by the generalization, so it set
$eachIsOversized to false and the merge was skipped. The generalized
members were then returned as separate union members.
- Since e606ca2 (unions of constant arrays with many keys no longer
degrade early), fetching an unknown offset of a big constant map, where
some entries are `[]` (bug-12671.php bench), hit this path. The union then
compared hundreds of generalized arrays pairwise. Their item types are
large unions of `list{...}&oversized-array`, so every comparison is itself
a union-vs-union check, which made the fetch quadratic (bench 0.3s -> 26s).
- Set empty constant arrays aside before generalizing, merge the remaining
members as before, and put the empty arrays back next to the result. A
union with an `array{}` member now gives `array{}` plus exactly what the
same union without it gives.
- Ported the same change to the native mirror in
turbo-ext/src/TypeCombinator.cpp.
- Probed the other paths that feed optimizeConstantArrays() (the
general-array branch of processArrayTypes() reuses the same function, so it
is covered). The inner value traversal already left empty arrays alone.
- Non-constant array members also prevent the merge. That is intended
(their item types are not generalized), so it is unchanged.
ondrejmirtes
force-pushed
the
create-pull-request/patch-sz9mpdv
branch
from
September 23, 2026 10:30
5827960 to
b940151
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
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.
Summary
The
bug-12671.phpbenchmark went from well under a second to ~26s. Bisecting pointed to e606ca2 ("Merge unions of constant arrays with many keys instead of degrading"). That commit exposed an existing weakness inTypeCombinator::optimizeConstantArrays(): a singlearray{}member kept the generalized oversized arrays from being merged into one. The union then compared hundreds of generalized arrays pairwise, each with a huge item-type union.With this change, analysing the bench file takes ~1.7s (0.8s with the turbo extension) instead of ~18.5s locally.
Changes
src/Type/TypeCombinator.php(optimizeConstantArrays()): empty constant arrays are set aside before the oversized generalization. The remaining members are merged as before ($eachIsOversizedis no longer spoiled byarray{}), and the empty arrays are put back next to the result.turbo-ext/src/TypeCombinator.cpp: same change in the native mirror. The regression test fails with the old native build and passes with the new one. Analysis output of the bench file is identical with and without the extension.processArrayTypes()goes through the same function, so it is covered.Root cause
The oversized generalization turns every non-empty
ConstantArrayTypeintonon-empty-array<K, V>&oversized-array. When all members were generalized, it merges them into one array. Empty arrays are deliberately not generalized (array{}&oversized-arraywould be contradictory), but that also left$isOversizedfalse for them, so$eachIsOversizedbecame false and the method returned every generalized member separately.In the benchmark,
$pr_states[$country]over a country→states map (many countries map to[]) produced ~250 such members. Their item types are unions oflist{code, name, localName}&oversized-array.TypeCombinator::union()then rancompareTypesInUnion()over them pairwise, and every comparison was a union-vs-unionisSuperTypeOf(). That made about 557k nestedisSuperTypeOfcalls for a single offset fetch.Before e606ca2, a hidden 62-distinct-key cap degraded such unions to a general array before reaching this code, which masked the problem.
The result is also more consistent now:
array{}|Xis exactlyarray{}plus what the union gives without the empty member. Previously the pairwise path relied on the intentionally lossy "oversized intersection is a subtype when maybe" rule inIntersectionType::isSubTypeOf(). For example, it absorbed Argentina's states into Spain's array and dropped them from the inferred type.Test
tests/PHPStan/Analyser/nsrt/bug-15293.phpbuilds a constant map of three entries with disjoint keys (together overConstantArrayTypeBuilder::ARRAY_COUNT_LIMITvalues) plus one[]entry. It asserts that an unknown-key fetch yieldsarray{}|(non-empty-array<…, list{…}&oversized-array>&oversized-array), identical to the map without the empty entry. Without the fix, the first assertion fails with a union of separatelist{n, n, n}&oversized-arraymembers. The existingtests/bench/data/bug-12671.phpcovers the performance side.Fixes phpstan/phpstan#15293
🤖 Generated with Claude Code