Shadow TypeTraverser in the turbo extension, binding its callables once per traversal instead of per node - #6424
Conversation
…once per traversal instead of per node
- Add turbo-ext/src/TypeTraverser.cpp — PHPStanTurbo\TypeTraverser, the native
counterpart of PHPStan\Type\TypeTraverser, with the same map() /
mapInternal() / traverseInternal() recursion over the unchanged userland
Type::traverse() implementations and callbacks.
- Mark PHPStan\Type\TypeTraverser with #[ShadowedByTurboExtension] and
PHPStan\Type\TypeTraverserCallable with #[ReferencedByTurboExtension], and
register the class-map key in turbo-ext/src/support.{h,cpp}, the
registration hook in main.cpp and the source in config.w32.
- The port absorbs, per visited node, the mapInternal()/traverseInternal()
frames and the [$this, ...] callable array the twin allocates for each of
them: both bound callables are built once per traversal and reused, then
dropped when map() returns so the traverser's lifetime stays tied to the
call.
- A TypeTraverserCallable is dispatched straight to traverse(), skipping the
adapter closure (and its per-node frame) the twin allocates for it.
- One traverser object is parked and reused by the next traversal, but only
when nothing outlived the previous one (refcount 1, not weakly referenced)
— a callback that kept its $traverse can still call it, and keeps its cb.
- Extend turbo-ext/tests/smoke.php with differential coverage over four type
shapes and six callback kinds (closure, non-traversing, TypeTraverserCallable,
array callable, first-class callable, nested map), plus exception
propagation, the non-Type return TypeError and an escaped traverse callable.
- Measured on a single-threaded self-analysis (810k map() calls, result cache
cleared): inclusive time inside map() drops from 1.270s to 1.09s, ~0.62% of
the 28.6s run; six interleaved whole-run A/B pairs all favour the port.
- Probed the structural sibling SimultaneousTypeTraverser: it is never called
during an analysis run, so it is left in PHP.
Closes phpstan/phpstan#15219
staabm
left a comment
staabm
left a comment
There was a problem hiding this comment.
revert all changes and shadow FileAnalyserCallback instead
…llables once per traversal instead of per node" This reverts commit 8484710.
|
Done. Here's what happened. 1. Reverted the PR's changes (as asked)
2. Shadowed
|
2 participants
Summary
The issue asks for
TypeTraverserto be shadowed by the phpstan_turbo extension: it shows up in profiles right next toNodeTraverser, which is already native.TypeTraverser::map()runs 810,046 times in a single-threaded self-analysis, visiting 1.41M nodes and making 1.30MType::traverse()calls. Almost all of that is per-node userland bookkeeping around two calls that have to stay in PHP (the user callback andType::traverse()): amapInternal()frame, atraverseInternal()frame, and a freshly allocated[$this, …]callable array for each of them, plus the traverser object itself and — for aTypeTraverserCallable— an adapter closure that adds one more frame per node.This PR implements the class natively and removes exactly that overhead, keeping the recursion, the callbacks and every
Type::traverse()implementation in PHP.Measured on the standard protocol (single-threaded, result cache cleared, user CPU, self-analysis of
src/Type):map(): 1.270 s → 1.09 s, i.e. ~0.18 s of a 28.6 s run — 0.62%, above the 0.5% bar the extension's design rules set for a port. Timing the traversal itself rather than the whole run is what makes an effect of this size resolvable: it is measured on the same instrumented region in both modes, and it reproduces to within 20 ms across runs.Changes
turbo-ext/src/TypeTraverser.cpp(new) —PHPStanTurbo\TypeTraverser, mirroring the twin method for method:map(),__construct()(private, as in the twin),mapInternal(),traverseInternal(). Registered through thereg::Classbuilder with raw handler pointers, state in the samecbproperty slot.[$this, 'mapInternal'],[$this, 'traverseInternal']) are built on first use and reused for every node of the traversal, then dropped whenmap()returns so the traverser's lifetime stays tied to the call — the twin allocates one array per node visit.TypeTraverserCallableis dispatched straight totraverse()viazend_call_known_instance_method(), instead of the closure the twin allocates in its constructor to adapt it (one allocation per traversal plus one frame per node).$traversekeeps a working traverser,cbincluded; nested traversals simply allocate their own, since the parked slot is empty while its object is in use. The parked object is released in RSHUTDOWN.map()instantiates the called scope — the stub subclassPHPStan\Type\TypeTraverser— so the traverser handed to callbacks is the class PHPStan's own code knows.src/Type/TypeTraverser.php—#[ShadowedByTurboExtension]. The PHP implementation is unchanged and stays the reference.src/Type/TypeTraverserCallable.php—#[ReferencedByTurboExtension(key: 'typeTraverserCallable')]; the native code needs the interface to recognise that form of callback.turbo-ext/src/support.h/support.cpp— thetypeTraverserCallableclass-reference entry, the registration/rshutdown hooks, andzend_closures.hin the shared engine include block.turbo-ext/src/main.cpp,turbo-ext/config.w32— registration hook, rshutdown hook, and the new source file for the Windows build (the Makefile andconfig.m4glob).turbo-ext/tests/smoke.php— differential coverage (see below).Parallel constructs probed
SimultaneousTypeTraverser— the exact structural sibling (samemap()/mapInternal()/traverseInternal()shape overType::traverseSimultaneously()). Instrumented over a full analysis run: zero calls. A site that is never hit can never pay for a port, so it stays in PHP.Closures instead of[$this, …]arrays would be worth another ~30% of the traversal's own cost on both sides, but it needs first-class callable syntax to be worthwhile (Closure::fromCallable()costs more than it saves at the real average of 1.7 visited nodes per traversal), andsrc/cannot use that syntax — the downgrade tooling has no visitor for it. Left alone so both implementations keep handing out the same kind of callable.Root cause
Not a bug — a missing port. The pattern is the one the extension exists for: a tiny class on a very hot path, where the userland cost is not the work itself but the frames and allocations wrapped around it.
TypeTraverserpays, per visited node, two userland frames and two array allocations that a native implementation can either absorb (the frames) or hoist out of the loop (the callables), plus one object allocation per traversal that can be reused. The user callback andType::traverse()— the actual work — keep running as PHP, so results are unchanged by construction.Test
turbo-ext/tests/smoke.phpgains aTypeTraversersection registered in$covered: four type shapes (leaf, array, union, nested intersection) × six callback kinds — closure (the documented constant-string-to-object example), a callback that never traverses, aTypeTraverserCallable, an array callable, a first-class callable, and a callback that starts a nestedmap()— each asserting the native and PHP results describe identically. Plus, for both implementations: an exception thrown by the callback propagates out ofmap(), a non-Typecallback result is aTypeError, and a$traversecallable the callback kept keeps working aftermap()returned (the case that must defeat object reuse).php turbo-ext/bin/side-by-side.php— 4 methods paired, generatedvendor/turbo-*files re-derived and byte-identical.php turbo-ext/tests/signature-parity.php— OK, 82 methods compared.make tests— 21361 tests green, both without the extension and with it loaded (identical assertion counts).make phpstan— green.--error-format=rawoversrc/Type, with and without the extension, diffs empty.gc_disable()add 0 bytes and no GC roots, matching the PHP implementation.Note for merging:
turbo-ext/src/changed, so this needs the usual follow-upmake bump-turbocommit once the change lands on the target branch — until then the extension version does not match and the extension stays inactive.Fixes phpstan/phpstan#15219