Answer UnionType comparisons from an identity-keyed FiniteTypeSet instead of scanning every member - #6116
Open
phpstan-bot wants to merge 4 commits into
Open
Conversation
… instead of scanning every member * Add `PHPStan\Type\FiniteTypeSet`: a union's members indexed by value identity - null, constant scalars other than floats, and bare enum cases. Members that cannot be keyed are kept aside so one object type next to fifty constant strings does not defeat the lookup. * `UnionType` builds the set lazily once per instance and answers `isSuperTypeOf()`, `isSubTypeOf()`, `accepts()`, `isAcceptedBy()`, `equals()` and `tryRemove()` from it, turning O(n*m) member comparisons into O(n+m) and single-value lookups into O(1). * `accepts()` cannot read a negative answer off the map (scalar coercion is not value identity), so for a value the union does not hold it consults one member per kind instead of all of them - members of a kind answer `accepts()` alike for a value none of them holds. * `TypeCombinator::finiteUnionMembers()` now uses the same cached set instead of keying the union again on every `intersect()`, keeping its class-string bail via `FiniteTypeSet::hasClassStringMember()`. * Same treatment for every finite kind, not just constant strings: constant ints, bools, null and enum cases share the keying. Floats stay out - `equals()` does not agree with value identity for them (-0.0 === 0.0, NAN !== NAN). * `SkipTestsWithRequiresPhpAttributeRule` checked for `PHP_VERSION_ID` on the left of the comparison only after demanding an inverse operator, so a test method starting with an `if` on any other binary operator crashed the analysis. Ask about `PHP_VERSION_ID` first. * New `FiniteTypeSetTest` compares every one of those operations against a reference implementation of the member-by-member loop, over a matrix of union and query types. `tests/bench/data/big-constant-string-union.php` covers it in the benchmark suite.
staabm
reviewed
Jul 28, 2026
… type A union of finite values is made of `ConstantStringType`, `ConstantIntegerType`, `ConstantBooleanType`, `NullType` and `ConstantFloatType`, and every comparison against such a union derives at least one key - so `key()` now answers for those five straight from `get_class()`. Matching the class exactly is what makes it safe: for these, `getConstantScalarTypes()` is `[$this]` and `equals()` is value identity, which is precisely what the general path checks before keying. Subclasses - template constant types among them - do not match and still walk it. That drops the three virtual calls and the array allocation the general path costs. The other allocation was `keyAndKind()`'s pair: the kind it carried is just the member's class - the property the kind stands for, that members of one kind answer `accepts()` identically, holds because they are the same class - so `kind()` derives it separately and `key()` returns a bare string. Only `create()` and `getRepresentativesOfOtherKinds()` ever ask for a kind. On 400-member unions (`UnionType` API directly, before -> after): | operation | before | after | | --- | --- | --- | | `key()` | 0.40 us | 0.12 us | | `isSuperTypeOf(single value)` | 0.65 us | 0.33 us | | `create()` | 175 us | 76 us | The set is only an optimization, so the guard is that it never disagrees with comparing member by member: disabling the shortcut leaves all 2698 cases of the existing matrix passing, which is what says it is a pure shortcut. Added on top: the shortcut classes are asserted to satisfy the checks it skips, a subclass is asserted to reach the general path and land on the same key, each kind is asserted to be represented once, and the matrix gained strings differing only in case - without those, case-folding the key went unnoticed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`containedIn()` asked the other set about one member at a time, so a union-vs-union comparison still cost a PHP-level call per member. The keys are what both sets are indexed by, so the whole comparison is a single C-level hash join. It asks for what is missing rather than for what is shared because that makes the yes answer the cheap one - `array_diff_key()` collects nothing when every member is present - and yes is the answer all three callers are after (`isAcceptedBy()` and `equals()` want nothing else). The no/maybe split then comes from the same count, no second pass. On 400-member unions, `isSubTypeOf()` goes from 20.1 ms to 4.3 ms for identical unions and from 17.4 ms to 6.7 ms for disjoint ones (2000 iterations). Covered by the existing matrix: making `containedIn()` answer no where it should answer maybe fails 43 of its cases. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
Comparing a union of constant strings against another one cost one
isSuperTypeOf()call perpair of members. Membership among such values is exact-value equality, so it is a set lookup:
this PR keys a union's members by value identity once, caches that on the union, and answers
isSuperTypeOf(),isSubTypeOf(),accepts(),isAcceptedBy(),equals()andtryRemove()from the map. O(n*m) member comparisons become O(n+m), and a single-value lookup becomes O(1).
The issue asked for a trie. A trie earns its keep on prefix or pattern queries — a router
matching URIs — and none of these comparisons are that (
ConstantStringType::isSuperTypeOf()on another constant string is
value === value). As noted in the issue's comment thread, anidentity-keyed hash is the structure this actually wants, and it answers the same question for
constant ints, bools,
nulland enum cases, so all of them are keyed, not just strings.Measured on 400-member unions (
UnionTypeAPI directly, before → after):isSuperTypeOfisSubTypeOfacceptsisAcceptedByisSuperTypeOf(single value)End to end, analysing
tests/bench/data/big-constant-string-union.phpgoes from 1.46 s to0.76 s. PHPStan's own self-analysis is unchanged (55.8 s → 55.4 s), so ordinary code pays
nothing for the map.
Changes
src/Type/FiniteTypeSet.php(new) — a union's members indexed by value identity. Keysnull, constant scalars other than floats, and bare enum cases; everything else goes to$others, so a single object type next to fifty constant strings does not defeat theoptimization, it only means callers still consult those few members the slow way. Also
exposes
containedIn()for union-vs-union answers andhasClassStringMember()for callersthat hand a member back rather than only comparing values.
src/Type/UnionType.php— builds the set lazily once per instance(
getFiniteTypeSet()) and uses it inisSuperTypeOf(),isSubTypeOf(),accepts(),isAcceptedBy(),equals()andtryRemove().src/Type/TypeCombinator.php—finiteUnionMembers()reuses the shared, cached setinstead of keying the union again on every
intersect(). Its class-string bail is kept: theclass-string flag is part of a constant string's representation but not of its value, and
intersect()hands a member back.build/PHPStan/Build/SkipTestsWithRequiresPhpAttributeRule.php— the rule demanded aninverse comparison operator before checking that the comparison was about
PHP_VERSION_IDat all, so a test method whose first statement is an
ifon any other binary operator (a||chain, say) crashed the analysis withShouldNotHappenException. The new test file hitit. The
PHP_VERSION_IDcheck now comes first.tests/bench/data/big-constant-string-union.php(new) — benchmark input.Root cause
UnionTypeanswered every comparison by walking$typesand asking each member, and thecallers that pass a union walk it again — so union-vs-union was O(n*m) virtual calls. For
members that stand for one concrete value that scan is pure waste: two of them are
interchangeable iff they are
equals(), and any two that are notequals()are disjoint. Akey derived from the value settles both questions with one hash lookup.
The same shape repeated across the whole comparison family, which is why the fix does too:
isSuperTypeOf(value)— yes if the map holds it; otherwise every keyed member answers no,so only unkeyed members are left to ask.
isSubTypeOf(union)/equals(union)— set containment between the two maps.accepts(value)— only the positive answer is value identity; a member that does not holdthe value can still accept it through scalar coercion. So the miss is not simply no, but it
is the same for every member of a kind (all constant strings behave alike as acceptors, and
so do all cases of one enum), and consulting one member per kind is enough.
accepts(union)— a member standing for a single value never accepts more than one of theother union's values, so the member-by-member
or()cannot come out yes and its result isdiscarded for the compound answer anyway; go straight to
isAcceptedBy().tryRemove(value)— drop the one member under that key instead of runningTypeCombinator::remove()against every member.Floats are the one finite scalar left out on purpose:
equals()does not agree with valueidentity for them (
-0.0 === 0.0,NAN !== NAN). Template types are excluded outright, theircomparison semantics are not value identity, and a type that merely contains a value — an
intersection with an accessory type, a whole single-case enum, a conditional type resolving to
a constant — is excluded by an
equals()check against its own constant scalar type.Test
tests/PHPStan/Type/FiniteTypeSetTest.php. The map is an optimization, so the property worthtesting is that it never disagrees with comparing member by member: each test runs the real
operation against a reference implementation that spells out the loop
UnionTypeused before,over a matrix of 17 unions × 18 query types (and 17 × 17 union pairs) — 2397 cases covering
constant strings, ints, bools,
null, enum cases, mixed-kind unions, unions diluted with anobject / a float / a general
string/ a class-string constant, andBenevolentUnionType.accepts()additionally asserts that no member attaches a reason to its answer, which is whatlets one member of a kind stand in for the rest without changing error messages.
The reference implementations caught two real mistakes while writing them (the
accepts()compound-delegation tail and
BenevolentUnionType::isAcceptedBy()'s or-semantics), andmutating the fix afterwards — inverting the
accepts()union shortcut, dropping thecompleteness guard in
isSuperTypeOf(), dropping theequals()guard inFiniteTypeSet::key()— fails 254, 6 and 12 cases respectively.Being an optimization, none of this fails without the fix; the benchmark data file and the
numbers above are what show the change did something.
make tests(20029 tests),make phpstanandmake csare green.make name-collisionfails identically with and without thechange (
tests/PHPStan/Build/data/final-class-rule-pipe.phpuses PHP 8.5 pipe syntax that thedetector's parser rejects on this PHP version).
Probed and deliberately left alone:
TypeCombinator::union()already collapses constant scalars and enum cases throughdescribe-keyed maps, so it is O(n) — no change needed.
TypeCombinator::doRemove()'s finite-type loop is O(n*m), but for a union of values it isunreachable:
isSuperTypeOf()settles the removal before it, andtryRemove()now handlesthe hit.
IntersectionType, the composite sibling, gets nothing: its members are notmutually-disjoint values, so identity keying does not apply.
UnionType::getFiniteTypes()dedupes bydescribe(VerbosityLevel::cache()). Switching it toidentity keys would collapse a class-string constant with a same-valued plain one, which
describe()keeps apart — left as is.Fixes phpstan/phpstan#15008