Repository navigation
[perf] Skip phpdoc path in RenameClassRector for nodes without a docblock - #8623
Merged
Merged
Conversation
…lock The rule visits Expression, If_, FunctionLike, Property and ClassLike nodes. For every one it built a PhpDocInfo and ran the phpdoc rename, even though only a real docblock can carry a renamable type. On rector-src src+rules that phpdoc path fired 28k times but only ~4k nodes had a docblock. Guard it with a cheap getDocComment() check so createFromNode and refactorPhpDoc run only when there is a docblock. Also drop the per-call serialize()+hash in createOldToNewTypes; the rename map is constant during a run, so memoize on the identical array and remove the now unused FileHasher dependency.
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
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.
Why
RenameClassRectoris one of the heaviest rules. It subscribes toFullyQualified,Name,Property,FunctionLike,Expression,ClassLikeandIf_, and for every non-name node it built aPhpDocInfoand ran the phpdoc rename, regardless of whether the node had a docblock.A phpdoc type rename can only ever change a node that carries a real docblock. Plain line comments or no comment never match.
Measurement
Single-process run over this repo's
src/+rules/with a 600-entry rename map, counting the phpdoc path:createFromNodewas called 28258 times (everyExpression/If_/FunctionLike/Property/ClassLikenode)refactorPhpDoc, which cannot change anythingThe phpdoc path (
createFromNode+refactorPhpDoc) accounted for ~92% of the rule's own time.Change
getDocComment() !== nullcheck. The expensive path now runs on ~4k nodes instead of ~28k. Behaviour is identical: nodes without a docblock produced an empty/typelessPhpDocInfothatrefactorPhpDocalways left unchanged.serialize()+ hash increateOldToNewTypes. The rename map is constant during a run, so memoize on the identical array and remove the now unusedFileHasherdependency.Tests
rules-tests/Renaming- 105 tests green