Repository navigation
[perf] Cache resolved used names per version in UnusedImportRemovingPostRector - #8702
Merged
Merged
Conversation
…ostRector To find unused `use` imports, enterNode() resolves every referenced name via two full-AST traversals (findNonUseImportNames + findNamesInDocBlocks), and that resolution was recomputed on every call. The post-rector chain re-runs this node across the file convergence iterations, so on iterations where the stmts did not change the same names were resolved again from scratch. Cache the resolved names per (file, stmts version): a new version re-resolves, an unchanged one reuses the previous result. The cache is reset when the file changes, so it never leaks across files or serves a stale map. Extends bench/unused_import_removing_bench.php (sets the file so the cached path is exercised).
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.
Why
To find unused
useimports,UnusedImportRemovingPostRector::enterNode()resolves every name referenced in the file via two full-AST traversals (findNonUseImportNames+findNamesInDocBlocks). That resolution was recomputed on every call.The post-rector chain re-runs this node across the file convergence iterations. On the iterations where the stmts did not change, the exact same name set was resolved again from scratch.
What
Cache the resolved names per
(file, stmts version):getNewStmtsVersion()re-resolves (stmts changed - the referenced names may differ)Removing unused
usestatements does not change the referenced-name set, and any rule that does change the body bumps the stmts version - so the version key stays correct.Bench
bench/unused_import_removing_bench.phpre-enters the same unchanged node (stable version), which is the ceiling case where every re-entry after the first is a cache hit. 40 repeats x 400 enters:The bench shows the ceiling; the real-world saving is the resolve cost on every convergence iteration after the first where the stmts are unchanged.
Tests
tests/Issues(incl.NamespacedUse,NoNamespaced,NamespacedUseAutoImport),rules-tests/CodingStyle,rules-tests/Renamingall green. ECS and PHPStan level 8 clean on the changed file.