flatten deep BooleanOr chains in TypeSpecifier to avoid O(n^2) recursion#5324
Conversation
|
You've opened the pull request against the latest branch 2.2.x. PHPStan 2.2 is not going to be released for months. If your code is relevant on 2.1.x and you want it to be released sooner, please rebase your pull request and change its target to 2.1.x. |
SanderMuller
force-pushed
the
perf/flatten-boolean-or-typespecifier
branch
from
5660bcb to
f8fcdc1
Compare
|
please add your benchmark in tests/bench/data |
|
CI benchmark results
|
src/Analyser/TypeSpecifier.php
Outdated
|
|
||
| // For deep BooleanOr chains, flatten and process all arms at once | ||
| // to avoid O(n^2) recursive filterByFalseyValue calls | ||
| if (BooleanAndHandler::getBooleanExpressionDepth($expr) > 8) { |
There was a problem hiding this comment.
This should reference the same constant the other places do
There was a problem hiding this comment.
Updated. What do you think about 4 vs 8 as the threshold?
… O(n^2) recursion
SanderMuller
force-pushed
the
perf/flatten-boolean-or-typespecifier
branch
from
f8fcdc1 to
56511e8
Compare
|
Thank you. |
|
BTW I hope this will all be obsolete once I finish #5224. Feel free to read the description. |
4 participants
Problem
Long
||chains like'A' === $x || 'B' === $x || ... || 'Z' === $x(common in validation logic, WordPress tag checks, etc.) cause quadratic performance inTypeSpecifier::specifyTypesInCondition().PHP's left-associative AST turns an 80-arm chain into a depth-80 tree. At each level,
filterByFalseyValue($expr->left)recurses intospecifyTypesInCondition()on the left subtree, which makes its ownfilterByFalseyValue()call, and so on. The call count grows as O(n^2).BooleanOrHandler::processExpralready has aBOOLEAN_EXPRESSION_MAX_PROCESS_DEPTH = 4limit for this reason, butTypeSpecifierhad no equivalent.On
tests/bench/data/bug-14207.php(153 lines, 80+||arms): ~3,000ms to analyse, with 20kfilterByFalseyValueand 40kspecifyTypesInConditioncalls.Solution
When depth exceeds 8, flatten the chain into leaf expressions and process each against the original scope in O(n):
SpecifiedTypesSpecifiedTypesTrade-offs
===comparisons this produces identical results, but interdependent conditions across arms lose some precision.processBooleanSureConditionalTypes/processBooleanNotSureConditionalTypes) are not computed for the flattened path, so cross-arm conditional type narrowing is lost for deep chains.BooleanOrHandleruses 4; we use a higher value here since the TypeSpecifier path produces richer type information worth preserving for moderate chains.Impact
bug-14207 (80+
||chain): 3,056ms → 1,529ms (-50%) (local benchmark)