Skip to content

[perf] Avoid re-parsing current file in RemoveUnusedPrivateClassConstantRector - #8644

Merged
TomasVotruba merged 3 commits into
rectorphp:mainfrom
guillaume-sainthillier:feat/rector-benchmark-optimize-a870ca
Oct 12, 2026
Merged

TomasVotruba merged 3 commits into
rectorphp:mainfrom
guillaume-sainthillier:feat/rector-benchmark-optimize-a870ca

Conversation

@guillaume-sainthillier

@guillaume-sainthillier guillaume-sainthillier commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Problem

ClassConstManipulator::hasClassConstFetch() searches for constant fetches in every ancestor of the class, resolved through AstResolver::resolveClassFromClassReflection(). ClassReflection::getAncestors() includes the class itself. So for every file that has a private constant, the file currently being processed was parsed again and its PHPStan scope fully resolved again, only to look for self::FOO in a class Rector already has in memory. The AstResolver cache doesn't help, because each file is processed once.

In a profile of Rector running its own rector.php on part of src/, this single call path accounted for ~16% of total runtime.

Change

  • ClassConstManipulator::hasClassConstFetch() now requires the ClassLike owning the constant, and searches only that in-memory class and the traits it uses. Parents and interfaces are no longer resolved, as a private constant cannot be fetched there. AstResolver is kept only for traits, since a trait method runs in the using class scope (covered by skip_used_in_trait.php.inc).
  • RemoveUnusedPrivateClassConstantRector now subscribes to Class_/Enum_ so it can pass the class node. Unused constants are collected first and removed afterwards, so the result doesn't depend on declaration order (as before, when the search ran on the unchanged file from disk).

Benchmarks

Single process, --dry-run --clear-cache, local machine:

Workload Before After
Rector's own rector.php (all prepared sets) on src/ (510 files) ~22.5s wall ~20.3s wall (−10%)
This rule only, ~3,400 vendor files (symfony, doctrine, nette, phpunit, …), parallel 27.5s / 167 CPU-s 19.5s / 132 CPU-s (−29% wall)

Behavior

  • On the 3,400-file corpus, the code change Rector produces is identical before and after.
  • Only the reported change metadata differs: since the rule now runs on the class node, the JSON changes entry points to the class line, with one entry per class instead of one per constant.
  • All existing fixtures pass unchanged. Full test suite: 5,016 tests OK.

…antRector

ClassConstManipulator::hasClassConstFetch() looks for constant fetches in
every class ancestor resolved through AstResolver. ClassReflection::getAncestors()
includes the class itself, so for each file with a private constant, the file
being processed was parsed again and its PHPStan scope fully resolved again.

The rule now subscribes to Class_/Enum_ and passes the in-memory class node,
which is searched instead of the re-parsed one. Unused constants are removed
only after all constants are checked, so the result does not depend on their order.
Comment thread src/NodeManipulator/ClassConstManipulator.php Outdated
A private constant cannot be fetched from a parent class or an interface,
so only the traits still need to be resolved through AstResolver.
Comment thread src/NodeManipulator/ClassConstManipulator.php
@TomasVotruba

Copy link
Copy Markdown
Member

Thanks, lets ship it 👍

@TomasVotruba
TomasVotruba merged commit f4c1ea1 into rectorphp:main Oct 12, 2026
49 checks passed
@guillaume-sainthillier
guillaume-sainthillier deleted the feat/rector-benchmark-optimize-a870ca branch October 12, 2026 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants