Skip to content

Index already normalized union members and overwrite markers instead of scanning them - #6969

Merged
ondrejmirtes merged 3 commits into
2.3.xfrom
faster-or-chain-bug-14869
Nov 6, 2026
Merged

ondrejmirtes merged 3 commits into
2.3.xfrom
faster-or-chain-bug-14869

Conversation

@ondrejmirtes

Copy link
Copy Markdown
Member

Makes tests/bench/data/or-chain-resolve-type-blowup.php and tests/bench/data/bug-14869.php cheaper to analyse, with the same inferred types and errors. Three commits; each one changes the PHP class and its turbo-ext mirror together.

Causes

  • TypeCombinator::doUnion(): skipping pairs of members that come from the same flattened union scanned every flattened union for every pair (isAlreadyNormalized()), so normalizing a union of k members cost O(k³). ObjectWithoutClassType::subtract() rebuilds object~(C1|…|Ck) with a growing k at every || arm, so this took about 65 % of the or-chain benchmark. The cost predates 2.3.
  • Guard derivation from d6831e9: since "Derive conditional guard types from the branch set difference", every branch merge calls TypeCombinator::remove($ourType, $theirType). The number of conditional holders created is the same before and after that commit; I counted 43,439 holders after merges in bug-14869 and 0 in or-chain on both sides. The extra cost is the remove() calls themselves. In bug-14869, half of them are remove(k, 0|1|…|k-1), which removed the union one member at a time, with a full remove() per member, only to return k unchanged.
  • VariableLivenessResolver from b1c223b: compileOverwrites() stored, for every write, every other write's overwrite marker twice (once as observed, once as killed). A variable assigned W times therefore cost O(W²) in memory and time, and the backward walk then unset all W markers at each write.

Changes

  • Look up already normalized union members by object identity: the members of the flattened unions are indexed by object id once per union, and only when the pairwise loop runs. Each pair is then a hash lookup. In C++, the index is a sorted vector of (object, union) pairs.
  • Remove a union from a value it does not hold with one lookup: in doRemove(), when the value being removed from has a FiniteTypeSet key and the union's set is complete, a key the set does not hold proves every member disjoint. The value then comes back unchanged without the per-member walk.
  • Answer overwrite markers against the live set instead of per write: only the markers are compiled, by variable and slot. When the backward walk reaches a write, it intersects the live set with the markers of that write's slots, walking whichever of the two is smaller. The live sets are bit-identical to before, so what is reported does not change.

Instruction counts

Valgrind cachegrind instructions for a full bin/phpstan analyse run (level 8 + bleedingEdge, OPcache on, JIT off), minus the same run on an empty file at the same commit:

file 2.3.x (bcca2b0) this PR (7fdc222) change
or-chain-resolve-type-blowup.php 10,374.7 M 3,696.3 M −64.4 %
bug-14869.php 2,249.6 M 2,172.5 M −3.4 %

Per commit, measured on the base before the rebase:

or-chain bug-14869
doUnion object-id lookup −6,679 M ±0
doRemove FiniteTypeSet lookup ±0 −43 M
overwrite markers against the live set ±0 −34 M

For reference, 2.2.17 measured 9,322.9 M for or-chain and 1,945.3 M for bug-14869 in the same setup. The rest of the bug-14869 gap comes from other commits.

Wall-clock time

bin/phpstan analyse -c <level 8 + bleedingEdge config> --debug --no-progress <file>, run with hyperfine: 2 warm-up runs, then 10 runs per side. The DI container cache was warm, and the result cache was cleared before every run (--prepare "bin/phpstan clear-result-cache"), so each run really analyses the file. Machine: Amp orb (E2B sandbox), 16 vCPU Intel Xeon @ 2.60 GHz; PHP 8.4.27 NTS, no turbo extension. tests/bench/phpstan.neon doesn't exist, so the config is the one sweep.sh uses.

file 2.3.x median this PR median change
_empty.php (start-up only) 0.879 s 0.873 s noise
or-chain-resolve-type-blowup.php 1.850 s 1.173 s −37 % (−69 % net of start-up)
bug-14869.php 1.184 s 1.201 s within noise

The sandbox's wall clock varies by about ±10 % (bug-14869 ranges: 1.134–1.214 s vs 1.145–1.255 s). The 3.4 % instruction-count gain on bug-14869 is below that noise level.

Tests

  • make tests passes: 23,472 tests, both without and with phpstan_turbo loaded (through PHP_INI_SCAN_DIR). DirectoryWalkerTest::testReplacedDirectoryIsReadAgain flaked once with the extension before the rebase, then passed on reruns; the test is unrelated to this change.
  • turbo-ext: tests/smoke.php ALL OK, tests/signature-parity.php OK (18,467 members compared), bin/side-by-side.php method parity OK. I regenerated only generated/TypeCombinator.h and generated/VariableLivenessResolver.h. In this environment (PHP 8.4), side-by-side already reports 59 other generated headers as stale on 2.3.x itself.
  • make phpstan reports no errors; phpcs is clean on the changed files.
  • Differential check: analysis output at level 4 + bleedingEdge over src/, tests/PHPStan/Analyser/nsrt and the DeadCode/Variables/Functions/Methods rule data (10,909 error lines) is byte-identical to 2.3.x. This holds both with and without the extension. The only turbo-mode difference is four PHPStanTurbo\Runtime class.notFound lines, which disappear because the extension provides that class. I excluded files that don't parse on PHP 8.4, plus nsrt/native-types.php, which crashes MissingReturnRule on 2.3.x too.
  • No new tests: the two benchmark files already cover these paths.

doUnion() skips comparing two members that come from the same flattened
union, but finding that out scanned every flattened union for each pair,
so normalizing a union of k members cost O(k^3). Repeatedly subtracting
classes from object (object~(C1|...|Ck) growing by one class per `||`
arm) hit this on every step.

The members are now indexed by object id once per union - only when the
pairwise loop runs - and each pair is answered by a hash lookup.

Amp-Thread-ID: https://ampcode.com/threads/T-01a1b134-fc8f-7115-81cd-f19bb1fdbf46
Removing a union peels its members off one at a time, a full remove() per
member, even when the value removed from is a constant none of them
stands for. Deriving conditional guards from the branch set difference
asks exactly that at every branch merge of a growing constant chain
(remove(3, 0|1|2)).

When the value has a FiniteTypeSet key and the union's set is complete,
a key the set does not hold proves every member disjoint, so the value
comes back unchanged without the per-member walk.

Amp-Thread-ID: https://ampcode.com/threads/T-01a1b134-fc8f-7115-81cd-f19bb1fdbf46
Each write that replaces a variable leaves a marker the previous
replacing write of the same slot observes and kills. compileOverwrites()
materialized, for every write, the list of every other write's marker
it observes and kills, so a variable assigned W times cost O(W^2) in
memory and time, and the backward walk then unset all W markers at each
write.

Only the markers are compiled now, by variable and slot. A write
intersects the live set with the markers of its slots when it is
reached, walking the smaller of the two, so the live sets, and with them
what is reported, stay the same.

Amp-Thread-ID: https://ampcode.com/threads/T-01a1b134-fc8f-7115-81cd-f19bb1fdbf46
@ondrejmirtes
ondrejmirtes force-pushed the faster-or-chain-bug-14869 branch from 7fdc222 to da9d917 Compare November 6, 2026 16:34
@ondrejmirtes
ondrejmirtes merged commit 50d7b35 into 2.3.x Nov 6, 2026
556 checks passed
@ondrejmirtes
ondrejmirtes deleted the faster-or-chain-bug-14869 branch November 6, 2026 16:40
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.

1 participant