Skip to content

Answer UnionType comparisons from an identity-keyed FiniteTypeSet instead of scanning every member - #6116

Open
phpstan-bot wants to merge 4 commits into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-7fiq36b
Open

Answer UnionType comparisons from an identity-keyed FiniteTypeSet instead of scanning every member#6116
phpstan-bot wants to merge 4 commits into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-7fiq36b

Conversation

@phpstan-bot

Copy link
Copy Markdown
Collaborator

Summary

Comparing a union of constant strings against another one cost one isSuperTypeOf() call per
pair 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() and tryRemove()
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, an
identity-keyed hash is the structure this actually wants, and it answers the same question for
constant ints, bools, null and enum cases, so all of them are keyed, not just strings.

Measured on 400-member unions (UnionType API directly, before → after):

operation identical unions disjoint unions
isSuperTypeOf 204 ms → 8 ms 575 ms → 6 ms
isSubTypeOf 203 ms → 6 ms 575 ms → 6 ms
accepts 2139 ms → 7 ms 2355 ms → 13 ms
isAcceptedBy 1504 ms → 6 ms 1731 ms → 12 ms
isSuperTypeOf(single value) 240 ms → 2 ms

End to end, analysing tests/bench/data/big-constant-string-union.php goes from 1.46 s to
0.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. Keys
    null, 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 the
    optimization, it only means callers still consult those few members the slow way. Also
    exposes containedIn() for union-vs-union answers and hasClassStringMember() for callers
    that hand a member back rather than only comparing values.
  • src/Type/UnionType.php — builds the set lazily once per instance
    (getFiniteTypeSet()) and uses it in isSuperTypeOf(), isSubTypeOf(), accepts(),
    isAcceptedBy(), equals() and tryRemove().
  • src/Type/TypeCombinator.phpfiniteUnionMembers() reuses the shared, cached set
    instead of keying the union again on every intersect(). Its class-string bail is kept: the
    class-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 an
    inverse comparison operator before checking that the comparison was about PHP_VERSION_ID
    at all, so a test method whose first statement is an if on any other binary operator (a
    || chain, say) crashed the analysis with ShouldNotHappenException. The new test file hit
    it. The PHP_VERSION_ID check now comes first.
  • tests/bench/data/big-constant-string-union.php (new) — benchmark input.

Root cause

UnionType answered every comparison by walking $types and asking each member, and the
callers 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 not equals() are disjoint. A
key 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 hold
    the 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 the
    other union's values, so the member-by-member or() cannot come out yes and its result is
    discarded for the compound answer anyway; go straight to isAcceptedBy().
  • tryRemove(value) — drop the one member under that key instead of running
    TypeCombinator::remove() against every member.

Floats are the one finite scalar left out on purpose: equals() does not agree with value
identity for them (-0.0 === 0.0, NAN !== NAN). Template types are excluded outright, their
comparison 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 worth
testing 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 UnionType used 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 an
object / a float / a general string / a class-string constant, and BenevolentUnionType.
accepts() additionally asserts that no member attaches a reason to its answer, which is what
lets 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), and
mutating the fix afterwards — inverting the accepts() union shortcut, dropping the
completeness guard in isSuperTypeOf(), dropping the equals() guard in
FiniteTypeSet::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 phpstan and make cs are green. make name-collision fails identically with and without the
change (tests/PHPStan/Build/data/final-class-rule-pipe.php uses PHP 8.5 pipe syntax that the
detector's parser rejects on this PHP version).

Probed and deliberately left alone:

  • TypeCombinator::union() already collapses constant scalars and enum cases through
    describe-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 is
    unreachable: isSuperTypeOf() settles the removal before it, and tryRemove() now handles
    the hit.
  • IntersectionType, the composite sibling, gets nothing: its members are not
    mutually-disjoint values, so identity keying does not apply.
  • UnionType::getFiniteTypes() dedupes by describe(VerbosityLevel::cache()). Switching it to
    identity keys would collapse a class-string constant with a same-valued plain one, which
    describe() keeps apart — left as is.

Fixes phpstan/phpstan#15008

staabm and others added 2 commits July 27, 2026 20:46
… 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.
Comment thread src/Type/FiniteTypeSet.php
phpstan-bot and others added 2 commits July 28, 2026 06:41
… 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Optimize UnionType for cases in which all innerTypes are a constant string

2 participants