Skip to content

Fix two more super-linear analysis-time channels in straight-line code#6064

Merged
ondrejmirtes merged 4 commits into
2.2.xfrom
worktree-issue-14972-perf
Jul 18, 2026
Merged

Fix two more super-linear analysis-time channels in straight-line code#6064
ondrejmirtes merged 4 commits into
2.2.xfrom
worktree-issue-14972-perf

Conversation

@ondrejmirtes

Copy link
Copy Markdown
Member

Follow-up to #6063 (issue phpstan/phpstan#14972). After the conditional-expression scan fixes, two more mechanisms kept straight-line code super-linear. Both are replicated by generated 800-statement files and now analyzed with linear (or near-linear) marginal cost per statement.

1. Fibers parked forever by rule-synthesized nodes

Issue 14972's variant A ($out .= "row: " . $a[$i] . … ; ×800) was still at 3.8s with a ~2.7× doubling ratio, and the scope tables stayed tiny — different mechanism entirely. InvalidBinaryOperationRule (and siblings) clone the operator node, replace its operands with TypeExpr, and ask $scope->getType($clone). On a FiberScope that suspends the rule-callback fiber waiting for a before-scope that can never arrive, because the synthesized node is not part of the AST NodeScopeResolver walks. Consequences per statement: ~6 fibers parked until end of function, every storeBeforeScope() scanning the whole pending list (O(N²) identity compares), and a starved fiber pool allocating a fresh Fiber (own C stack) for nearly every rule callback.

  • ExpressionResultStorage::$pendingFibers is now keyed by spl_object_id() of the requested expression — resolving a before-scope touches only the fibers waiting for it. This also protects third-party rules that ask about synthesized nodes.
  • FiberScope::getType()/getNativeType() answer TypeExpr directly (scope-independent by construction).
  • InvalidBinaryOperationRule, InvalidUnaryOperationRule and AssertRuleHelper ask about their own synthesized nodes via getScopeType() — semantically identical to what the end-of-function fallback resumed the parked fiber with, minus the parking.

2. Invalidation-gate holes hit on every impure call

A new reproducer ($aN = $this->compute($N); ×800) exposed a quadratic staircase in shouldInvalidateExpression():

  • Every impure method call invalidates '$this' with $requireMoreCharacters, where the compositional substring gate is off — so every tracked expression, including every plain variable, went through the AST containment walk. A plain variable cannot contain more than itself, so it now short-circuits.
  • PossiblyImpureCallExpr entries (one accumulated per impure call) always slow-pathed the walk because their keys contain __phpstan. Their printer is compositional (children printed verbatim), so the new keyMayHideSubExpressions() exempts them (and AlwaysRememberedExpr) from the blanket exclusion.
  • invalidateMethodsOnExpression() applies the same gate before re-printing every tracked method call's receiver on every property assignment.

Both mirrored in the turbo extension's ScopeOps.cpp, with the follow-up expected-version bump.

Numbers (user CPU, N=100/200/400/800)

repro before after (PHP) after (turbo)
concat (variant A) 0.67 / 0.89 / 1.64 / 3.83s 0.74 / 0.95 / 1.25 / 1.61s 0.70 / 0.81 / 1.02 / 1.35s
impure-call columns 0.59 / 0.67 / 0.89 / 1.50s 0.59 / 0.67 / 0.83 / 1.20s 0.58 / 0.64 / 0.78 / 1.06s

Bench guards: tests/bench/data/bug-14972-concat.php, tests/bench/data/impure-call-columns.php.

Verification

  • Full test suite green with the extension shadowing active and with PHPSTAN_TURBO=0
  • Analysis output byte-identical between both modes, and before vs after the fixes, on the repro files and a src/Type self-analysis
  • make phpstan clean; turbo strict build, smoke.php, signature-parity.php, side-by-side.php --check all pass

Note for merging: a rebase-merge or squash-merge rewrites the SHA that TurboExtensionEnabler::EXPECTED_EXTENSION_VERSION points at (the bump commit expects 0ceb9aa), and will need the same post-merge correction as last time.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Gh8bVRSqtiLRW7ifukHdW8

ondrejmirtes and others added 4 commits July 18, 2026 21:34
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gh8bVRSqtiLRW7ifukHdW8
Rules that synthesize an expression node (clone with TypeExpr operands in
InvalidBinaryOperationRule/InvalidUnaryOperationRule, the asserted
expression in AssertRuleHelper) and ask the scope for its type suspended
their callback fiber waiting for a before-scope that NodeScopeResolver
can never store - the node is not part of the walked AST. Each such fiber
stayed parked until the end of the function, so in operator-heavy
straight-line code (issue 14972's variant A) the pending list grew by ~6
fibers per statement, every storeBeforeScope() call scanned all of them,
and the parked fibers starved the reuse pool, forcing a fresh Fiber (and
C stack) per callback.

- ExpressionResultStorage::$pendingFibers is now keyed by the requested
  expression's spl_object_id(), so resolving a stored before-scope touches
  only the fibers waiting for that expression instead of scanning all of
  them.
- FiberScope::getType()/getNativeType() answer TypeExpr directly - its
  type is scope-independent by construction.
- The rules above ask via getScopeType(), which resolves against the
  ask-time scope - exactly what the end-of-function fallback resumed the
  parked fiber with, minus the parking.

800 concat-assignment statements (tests/bench/data/bug-14972-concat.php)
drop from 3.8s to 1.6s user time, and the marginal per-statement cost is
now linear.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gh8bVRSqtiLRW7ifukHdW8
Three refinements to the scope-invalidation scans, mirrored in the turbo
extension's ScopeOps.cpp:

- A tracked plain variable cannot contain more than itself, so with
  $requireMoreCharacters it can never match and the AST containment walk
  is skipped. This covers the '$this' invalidation that runs for every
  impure method call, where the substring gate is unusable - previously
  it walked every tracked expression.
- The blanket '__phpstan' exclusion from the substring gate now exempts
  virtual-node wrappers whose printers include all children verbatim
  (PossiblyImpureCallExpr, AlwaysRememberedExpr) via
  keyMayHideSubExpressions(). PossiblyImpureCallExpr entries accumulate
  one per impure call and previously slow-pathed through the containment
  walk on every invalidation.
- invalidateMethodsOnExpression() applies the same gate before re-printing
  a tracked method call's receiver, instead of printing it for every
  entry on every property assignment.

800 sequential impure-call assignments
(tests/bench/data/impure-call-columns.php) drop from 1.5s to 1.2s user
time with a near-linear tail.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gh8bVRSqtiLRW7ifukHdW8
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gh8bVRSqtiLRW7ifukHdW8
@ondrejmirtes
ondrejmirtes force-pushed the worktree-issue-14972-perf branch from e755af9 to 92a8d9c Compare July 18, 2026 20:37
@ondrejmirtes
ondrejmirtes merged commit 59de482 into 2.2.x Jul 18, 2026
734 of 737 checks passed
@ondrejmirtes
ondrejmirtes deleted the worktree-issue-14972-perf branch July 18, 2026 21:34
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