Repository navigation
[perf] Use native instanceof in NodeGroup::isStmtAwareNode - #8746
Merged
Merged
Conversation
isStmtAwareNode() is called per node on the traversal hot path (top call count in a full profile). It used array_any() over STMTS_AWARE with a closure doing a dynamic $node instanceof $class per element - a closure call plus dynamic instanceof for each of 18 classes. Replace with native instanceof via match(true): no per-element closure, no dynamic instanceof, same subclass semantics. bench/node_group_is_stmt_aware_bench.php (non-stmt-aware-heavy node mix): 3.34 -> 0.75 us/call (~4.5x).
Member
Author
|
LGTM 👍 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A fresh full-run profile (in-process, worker-faithful) put
NodeGroup::isStmtAwareNode()'s closure at the top of the call-count table - it is called per node on the traversal hot path.It used
array_any(self::STMTS_AWARE, fn ($class) => $node instanceof $class)- a closure call plus a dynamic$node instanceof $classfor each of the 18 classes, per node. Replaced with nativeinstanceofviamatch (true): no per-element closure, no dynamic instanceof, same subclass semantics (verified equal on every node type).match (true)is required in source: the active codeQuality set (SwitchTrueToMatchRector) rewrites anyswitch (true)back tomatch. For the publishedrector/rectorthe downgrade pipeline (DowngradeMatchToSwitchRector) converts it toswitch (true), so downgraded (PHP 7.2) users run the switch form - which is the fastest of the three.Numbers
Non-stmt-aware-heavy node mix (the worst case that exhausts every instanceof), CPU time, same run, 18.6M calls:
array_any+ closurematch (true)switch (true)Correctness verified identical across all node types for all three forms. Full
vendor/bin/phpunit(4990 tests) green, PHPStan and ECS clean.