Flatten deep BooleanAnd chains in resolveType() and the falsey context

[SanderMuller]

Closes phpstan/phpstan#15004

A long && chain of !== comparisons against a literal union is quadratic. The 400-clause reproducer from the issue goes from 18.4s of CPU to 1.0s, and the growth curve flattens to linear.

The report's root cause was wrong, and I wrote it

I reported this as the TypeCombinator::remove counterpart of the intersect blowup #5935 fixed, and proposed mirroring that fast path into remove. I built that first. It made things slightly slower (0.90x at N=400), and the counters say why: of 279,295 remove calls at N=200, the fast path could serve 40,381, and every one of them still had to rebuild the union — there was no cheap "value is not a member" case to win at all. It only added a keying attempt in front of the existing work. Dropped it.

Subtractive probe on where the time really goes: stubbing doRemove() to return its input removes 63% at N=400, but what remains still grows x4.54 per doubling. So remove is expensive but not the shape of the problem — the number of narrowing operations is, and that number is quadratic.

What it actually is

BooleanOrHandler flattens deep chains in both resolveType() and specifyTypes(). BooleanAndHandler flattened only specifyTypes(), and only when $context->true(). The two missing halves each recursed into the left operand and re-narrowed the whole left chain at every level.

The asymmetry is visible as a straight measurement — same reproducer, same method, only the operator differs (3 rounds, medians):

N	$x !== 1 && ...	$x === 1 || ...
100	0.93s	0.48s
200	3.17s	0.59s
400	18.38s	0.98s
per doubling	x3.41, x5.80	x1.24, x1.65

And instrumenting the handler on 2.2.x shows exactly which path runs: for the 200-arm chain the truthy side flattens 582 times, while the recursive path is entered 407 times at depths 193-198 — all of them in a falsey context (false() set, truthy() clear). That is the else side of the chain, plus resolveType.

The change

Both additions mirror their BooleanOr counterparts:

• resolveTypeForFlattenedBooleanAnd() threads the truthy scope arm by arm: false if any arm is false, true if every arm is true, bool otherwise.
• specifyTypesForFlattenedFalseyBooleanAnd() is the De Morgan mirror of the flattened truthy BooleanOr chain: at least one arm is false, so the arms' narrowings intersect. Like that path, a deep chain trades the per-pair conditional-holder augments for linear time.

Every non-true context that is not the null context takes the flattened falsey path. I first gated it more narrowly ($context->false() && !$context->truthy()) to keep mixed truthy-and-false contexts on the recursive path, and dropped that guard once the mutation gate showed it was unobservable — see the CI note below.

I measured the trade-off rather than leaning on the precedent: instrumenting 2.2.x to count how often the deep-falsey recursive path actually builds a non-null branchUnionAugment, it fires 3 times across the whole test suite and twice on the 1144-file corpus. So it is a real trade-off, not a proven no-op - but in both samples the resulting output is unchanged (suite green, corpus byte-identical). If you would rather keep the augment for deep chains, it can be built once from the flattened arms instead; I did not do that because it reintroduces per-pair work and nothing measurable depends on it.

Numbers

Interleaved base/PR per N, 3 rounds, medians, level 8, single file, cold cache, CPU as user+sys, PHP 8.5.8:

N	2.2.x	PR	speedup
50	0.56s	0.48s	1.17x
100	0.93s	0.52s	1.78x
200	3.17s	0.63s	5.03x
300	8.56s	0.80s	10.67x
400	18.38s	1.04s	17.63x

Growth per doubling of N: x3.41 (100->200) and x5.80 (200->400) on base; x1.21 and x1.65 with this change. The PR's 1.04s at N=400 matches the || shape's 0.98s, so the two operators now cost the same.

The same holds for string literals rather than ints ($x !== 's1' && ...): 17.30s -> 1.01s at N=400, per-doubling x5.88 -> x1.63. Worth noting because the issue claimed the plain-string version was already handled by the flattening — it was not, it is equally quadratic on 2.2.x.

No regression

• Full suite green (21310 tests). Self-analysis clean, phpcs clean.
• On a real-world Doctrine/Symfony application reporting 3267 errors over 1144 files, output is byte-identical to 2.2.x, and CPU is 3.2% lower (medians of 2 interleaved rounds, 134.6s -> 130.3s).
• The reproducer reports the same 3 errors before and after.

Tests

nsrt/deep-boolean-and-chain.php pins the narrowing on both sides of chains longer than BOOLEAN_EXPRESSION_MAX_PROCESS_DEPTH, against a short chain on the recursive path for comparison, plus a negated chain, a mixed-arm chain whose falsey side cannot narrow, and a null-check chain. It passes on 2.2.x too — it is a characterisation test, since the change is meant to leave inference untouched.

tests/bench/data/and-chain-resolve-type-blowup.php adds the instanceof-arm shape, mirroring or-chain-resolve-type-blowup.php and following how 7eab3d2 added its bench in the same commit as the fix. It is a strong discriminator: 17.27s unflattened, 9.17s with only resolveType() flattened, 1.08s with both.

I first also added an and-chain-falsey-blowup.php and then deleted it, for two reasons worth stating: it was a near-duplicate of the existing and-chain-truthy-blowup.php (same 100-arm !== shape), and instrumenting confirmed that existing bench already enters the new flattened falsey path, so mine guarded nothing new. It is also the bench that shows this change most clearly in CI - and-chain-truthy-blowup.php comes out at -81.88% against the committed baseline.

Note the new variant will not be in the committed phpbench baselines until they are regenerated.

About the CI reds

• Benchmark / Test (PHP 8.5) flags bug-13352.php +11.09% and bug-14624.php +15.89% against the committed baseline. That is baseline drift, not this change: A/B on one machine, two rounds, medians, gives 2.07s vs 2.08s and 1.91s vs 1.91s - 1.00x for both. The same job is red on unrelated branches.

• Mutation Testing was red twice, and both were mine. Worth writing out, because the second one changed the code.

  First, two LooseBooleanMutator mutants. That mutator appends ->toBoolean() to isTrue()/isFalse() receivers, and its canMutate() deliberately skips a receiver that is already a ->toBoolean() call — but I had assigned that call to a variable first, so it could not see through it and produced two no-op mutants no test can kill. Fixed by inlining toBoolean() at both check sites.

  Then one TrueTruthyFalseFalseyTypeSpecifierContextMutator mutant on my narrower gate: $context->false() -> $context->falsey(). I could not kill it, and the reason is structural. CONTEXT_FALSEY is CONTEXT_FALSE | CONTEXT_FALSEY_BUT_NOT_FALSE, and a context with the second bit but not the first is not produced by any factory nor by negate(), so false() and falsey() agree on every reachable context. Instrumenting which contexts reach the depth gate across the whole suite gives only CONTEXT_TRUTHY (4x) and CONTEXT_FALSEY (7x); constructing mixed contexts on purpose ((chain) !== true, === false) reaches 0b1110, but !$context->truthy() masks the swap there too, so every phrasing of that guard yields an equivalent mutant.

  I checked whether the guard did anything before removing it: routing the mixed context through the flattened path instead produces identical inference on the shape I could build. So the guard was unobservable, and the gate is now just !$context->null() — null() is not a mutator target. The trade-off this widens is the same one the plain falsey path already makes (holders the recursive path re-derives), on a rarer context. Happy to restore the narrower guard and let that mutant escape if you would rather keep it.

[ondrejmirtes]

Please look at this in the context of my single pass PR if it's still relevant after it

[ondrejmirtes]

I'm talking about #5857

[SanderMuller]

Checked it against #5857, and you are right: this is not needed after it. Measured rather than assumed, same reproducer and method as the table above (level 8, single file, cold cache, CPU as user+sys, PHP 8.5.8):

N	2.2.x	this PR	#5857 (df01b1f)
50	0.56s	0.48s	0.47s
100	0.93s	0.52s	0.51s
200	3.17s	0.63s	0.66s
400	18.38s	1.04s	1.17s

#5857 is linear on it (per doubling x1.29, x1.77), so the blowup this PR targets is already gone there. The same holds for the string-literal variant ($x !== 's1' && ...): 17.30s on 2.2.x, 1.19s on #5857 at N=400. On the instanceof-arm bench I added, #5857 is 2.41s against 17.27s on 2.2.x.

It is not just redundant, it is inapplicable: #5857 deletes both methods this PR patches. BooleanAndHandler there is down to supports() and processExpr() — no resolveType(), no specifyTypes(), and BOOLEAN_EXPRESSION_MAX_PROCESS_DEPTH and the flattening are gone from src/ entirely. So there is nothing to rebase; it would have to be re-derived against the new architecture, and on these numbers there would be nothing left to fix.

So I would close this, and I am happy to do that — your call on one thing first: do you want the fix in a 2.2.x release before #5857 lands, or is #5857 close enough that it is not worth carrying? The report (phpstan/phpstan#15004) is about a 400-clause function costing 18s today, so the only argument for landing this is the gap until #5857 ships.

Two artifacts here are worth keeping either way, and I have verified both against #5857 rather than assuming:

• tests/PHPStan/Analyser/nsrt/deep-boolean-and-chain.php — pins narrowing on both sides of chains past the depth threshold, including chains compared against true/false. It passes unchanged on Single-pass expression analysis groundwork - answer type questions from ExpressionResults #5857, so it guards the new architecture too.
• tests/bench/data/and-chain-resolve-type-blowup.php — the instanceof-arm shape. There is already an and-chain-truthy-blowup.php, but not this one, and it is the shape that showed the blowup most clearly (17.3s on 2.2.x).

Say the word and I will open a separate PR with just those two against 2.2.x, and close this.

One correction to my own report while I am here, since it is wrong on the record: I filed phpstan/phpstan#15004 blaming TypeCombinator::remove and proposing a fast path mirroring #5935. I built that first and it was slower — the fast path could serve 40,381 of 279,295 calls at N=200 and every one of them still rebuilt the union, so there was no cheap case to win. The cause was the missing BooleanAnd flattening, which is what #5857 addresses structurally.

[ondrejmirtes]

I'll take the tests file myself into #5857, no further action required. I hope to release #5857 at some point in August or early September. After that I plan to rewrite the entirety of ExprHandlers and some surrounding code for the Turbo extension (if it's going to yield performance improvements).

Thank you.