Repository navigation
[perf] Resolve used names in a single AST walk in UnusedImportRemovingPostRector - #8704
Merged
Merged
Conversation
…gPostRector To find unused `use` imports, enterNode() resolved the referenced names with two separate full-AST traversals of the same stmts - findNonUseImportNames() for PHP names and findNamesInDocBlocks() for docblock class names. Merge both into a single traversal: each node's docblock names are collected in the same callback that gathers the PHP names. The Use_ subtree is still skipped for name collection (its node docblock is read first), and the root node's own docblock is read up front, so the resolved set is unchanged - the stmts are walked once instead of twice. Adds bench/unused_import_resolve_bench.php, which bumps the stmts version per call to force the walk each time and isolate the first-pass cost.
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.
Follow-up to #8701 / #8702. End-to-end profiling of the import post-rector chain (single-process, over rector-src's own
src/, 510 files) showedUnusedImportRemovingPostRectorwas ~61% of the remaining import cost - the guard (#8701) and the cross-pass cache (#8702) do not touch its first-pass walk cost.Why
To find unused
useimports,enterNode()resolved the referenced names with two separate full-AST traversals of the same stmts:findNonUseImportNames()- PHP namesfindNamesInDocBlocks()- docblock class namesWhat
Merge both into a single traversal. Each node's docblock names are collected in the same callback that gathers the PHP names. The resolved set is unchanged:
Use_subtree is still skipped for name collection (its node docblock is read first, before the skip)$rootNode->stmts, matching the former names walk exactly (so the namespace's own name is not collected)The stmts are walked once instead of twice.
Bench
Adds
bench/unused_import_resolve_bench.php, which bumps the stmts version per call to force the walk every time (no cache hits) and isolate the first-pass cost. 40 repeats x 400 forced walks onbench/Source/ManyNamesSource.php:~24% off the resolve walk.
Tests
tests/(701, engine + PostRector),tests/Issues(incl.NamespacedUse,NoNamespaced,NamespacedUseAutoImport),rules-tests/CodingStyle,rules-tests/Renamingall green. ECS and PHPStan level 8 clean on the changed file.