Skip to content

Serve node callbacks a synchronous storage-backed scope and drop the fiber resolver - #6248

Merged
ondrejmirtes merged 11 commits into
2.2.xfrom
drop-fnsr
Aug 23, 2026
Merged

ondrejmirtes merged 11 commits into
2.2.xfrom
drop-fnsr

Conversation

@ondrejmirtes

Copy link
Copy Markdown
Member

Drops the fiber-based node-callback machinery: rules on every PHP version now receive the same storage-backed scope, synchronously.

Why

FiberNodeScopeResolver existed to let a rule's type asks wait for the walk to store the asked expression's result — on PHP < 8.1 the plain resolver answered the same asks by re-walking on demand (~380k re-walks during self-analysis, +15% user CPU vs fibers). With node callbacks emitted after the node's results are stored, nothing needs to wait anymore: the callback scope answers every ask from the storage directly.

How

  • Emission reordering (3 commits): expression nodes emit their callback right after the handler's result is stored; expression-carrying statements (echo, return, expression statements, if/elseif/switch conditions, assignment targets, boolean operands) after their expressions are processed — always with the scope captured at the entry position, so rules observe the same (scope, answer) pair as before.
  • Synchronous answering + machinery removal: FiberScope answers stored asks by pricing on the stored before-scope (what the fiber resume delivered), filter-derived and promoted asks by re-reading on that scope, and unstored asks (synthetic nodes) on demand through the MutatingScope path (what the fiber flush delivered). The emitting walk's storage is bound for the duration of the callback through the new ExpressionResultStorageStack — scopes resolve it through the container and never reference a storage directly, which would cycle with the storage's stored scopes and never free with the cycle collector disabled. Fiber creation, parking, resuming and flushing goes away, including the mirror properties in the native ExpressionResultStorage.
  • One resolver for every PHP version: FiberNodeScopeResolver, FnsrExtension, the PHPSTAN_FNSR toggle and the PHP 8.1 gate are deleted; 7.4 and 8.0 get the same storage-backed rule scope with walk-position answers as everything else — the plain-mode re-walk cost is gone.
  • Renames: FiberScopeNodeCallbackScope (the Fiber namespace dissolves; the fnsr.php fixture moves under nsrt auto-discovery), toMutatingScope()toWalkScope() with a deprecated toMutatingScope() alias kept — extensions (phpstan-doctrine's OtherMethodQueryBuilderParser) call it. The native ScopeOps mirror follows the renamed properties.
  • Follow-ups: resetPerFileAnalysisState() moves to the per-file callers so extension-started nested walks stop wiping per-file caches mid-file; node-callback type asks are memoized by node identity, restoring the O(1) repeat asks the walk scope's own memo used to provide.

Behavior notes

Serving the walk's stored answers surfaces closure argument types the suspension flow priced naively before (first-ask-wins through scope-instance memos): preg_replace_callback callbacks returning $matches[0] under PREG_OFFSET_CAPTURE now correctly report the offset tuple leaking into the string return type — CallToFunctionParametersRuleTest expectations updated with the two new true positives.

Test suite is green with the turbo extension inactive and active with bit-identical results; cold-cache self-analysis clean in both modes.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GnwgpaeUXRkgSDyg95tfK8

Rules and DependencyResolver receive a node's callback and immediately ask
about the node or its subexpressions. Under fibers a pre-order callback
parks on its first ask and resumes when the natural walk stores the result
anyway - but a synchronously invoked callback (the plain resolver on
PHP < 8.1) re-walked everything it asked about through the on-demand
bridge: ~380k re-walks during self-analysis, +15% user CPU vs fibers.

Expression nodes now emit their callback right after the handler's result
is stored, and the expression-carrying statements (echo, return,
expression statements) after their expressions are processed - in both
cases with the scope captured at the entry position, so rules observe the
same (scope, answer) pair as before. Self-analysis on the plain resolver
drops from 470k to 107k on-demand walks; fibers are unchanged.

(cherry picked from commit 2848d8b)
…lts are stored

Continues the previous commit for the remaining synchronous-callback
re-walk clusters: if/elseif/switch emit their statement callback right
after the condition's result is stored (rules like the constant-condition
and boolean-in-condition helpers ask about the condition), and
prepareTarget() emits the raw assignment target's callback after the walk
composed and stored the target's read result (DependencyResolver and the
property rules ask about the target and its receiver). Scopes stay
captured at the entry position. Self-analysis on the plain resolver drops
from 107k to 79k on-demand walks - 14.6k of them on real nodes, down from
380k before the two commits.

(cherry picked from commit 71eadeb)
…h callback

The constant-condition rules listening on BooleanAndNode/BooleanOrNode ask
about the raw binary expression, and foreach rules about the iteratee.
The boolean handlers now store their result before emitting the virtual
node (the later store in processExprNodeInternal is an idempotent re-store
of the same result), and the foreach statement emits its callback after
the iteratee's result is stored, with the entry scope.

(cherry picked from commit 486e4a4)
Post-order emission stores the node's own result and every subnode result
before the callback fires, so FiberScope answers every ask from the
storage: stored asks by pricing on the stored before-scope, filter-derived
and promoted asks by re-reading on that scope with the filters applied,
unstored asks (synthetic nodes, nodes ahead of the walk) on demand through
the MutatingScope path - the same answer the fiber flush produced for a
never-stored ask. Node callbacks run directly; the fiber creation, parking,
resuming and flushing machinery is removed, including its mirror properties
in the native ExpressionResultStorage.

The emitting walk's storage is bound for the duration of the callback
through the new ExpressionResultStorageStack - scopes resolve it through
the container and never reference a storage directly, which would cycle
with the storage's stored scopes and never free with the cycle collector
disabled.

Serving the walk's stored answers also surfaces closure argument types the
suspension flow priced naively before: preg_replace_callback callbacks
returning $matches[0] under PREG_OFFSET_CAPTURE now correctly report the
offset tuple leaking into the string return type.
A rule may pass the scope it was handed - the rule-facing FiberScope - as
the initial scope of a processNodes()/processStmtNodes() walk (shipmonk's
ForbidCheckedExceptionInCallableRule re-walks callable bodies this way).
The walk then anchors its results to fiber scopes, and consuming such a
result from a filter-derived ask re-enters the rule-facing ask paths,
deriving scopes without end. The public entry points now normalize to the
state-identical MutatingScope, and preprocessScope() guards the
consumption side the same way.

(cherry picked from commit 53d5227)
FiberNodeScopeResolver's only remaining behavior - unwrap gatherers,
skip noop callbacks, hand rules the FiberScope - moves into the base
resolver, and the class goes together with the FnsrExtension autowiring
switch, the PHPSTAN_FNSR toggle, and the PHP 8.1 gate: nothing here
needs fibers anymore, so 7.4 and 8.0 get the same storage-backed rule
scope with walk-position answers as everything else. Test expectations
that keyed on the resolver split become unconditional.

(cherry picked from commit f5fede8)
Nothing about the class is fiber-specific anymore: it is the scope every
node callback receives, answering asks from the walk's stored expression
results. The Fiber namespace dissolves (the resolver override moved to
the base class earlier), toFiberScope() becomes toNodeCallbackScope(),
the scope factory flavour flag says what it creates, and the fnsr.php
type-inference fixture moves under nsrt/ auto-discovery, making its
dedicated test class redundant.

(cherry picked from commit 2efbec6)
The method answers "the scope the engine walk runs on" - for a walk scope
itself, and the state-identical MutatingScope for a NodeCallbackScope. The
native ScopeOps mirror follows the renamed properties.

(cherry picked from commit dab7c5d)
…e boundary only

toMutatingScope() returns $this and stays as a deprecated alias -
extensions (phpstan-doctrine's OtherMethodQueryBuilderParser) call it.

resetPerFileAnalysisState() moves from processNodes() to the per-file
callers (FileAnalyser, TypeInferenceTestCase): extensions start nested
processNodes() walks mid-file - phpstan-doctrine parsing a query-builder
method, rule tooling re-analysing a callee - and each wipe forced the
outer file to rebuild its per-file caches, re-converging closure types
and recomputing narrowing memos. On shipmonk's test files, where warm
reflection state makes those rebuilds expensive and query-builder
consults are frequent, whole analysis-order windows ran 2x slower than
2.2.x while the same files in isolation were near parity.

(cherry picked from commit 847b7f0)
Rules and collectors re-ask the same nodes across a callback batch, and
the walk scope's resolvedTypes memo used to answer those repeats in O(1)
before the callback-facing scope existed. Every repeat paid the
stored-result guard - variable-state compares, node-key printing on
re-priced asks - which shipmonk's rule set (disallowed-calls formatting
every call, the dead-code collectors) multiplied into whole test-file
windows running twice as slow as 2.2.x. The entry pins the asked node:
a dropped synthetic's object id can be reused by the next synthetic, and
the identity check rejects the stale hit.

(cherry picked from commit b50dfb4)
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