diff --git a/bench/class_reflection_refresh_bench.php b/bench/class_reflection_refresh_bench.php new file mode 100644 index 00000000000..b5cb850959b --- /dev/null +++ b/bench/class_reflection_refresh_bench.php @@ -0,0 +1,155 @@ +create(); +$rectorConfig->boot(); + +/** @var TestingParser $testingParser */ +$testingParser = $rectorConfig->make(TestingParser::class); + +/** @var BetterNodeFinder $betterNodeFinder */ +$betterNodeFinder = $rectorConfig->make(BetterNodeFinder::class); + +/** @var ChangedNodeScopeRefresher $changedNodeScopeRefresher */ +$changedNodeScopeRefresher = $rectorConfig->make(ChangedNodeScopeRefresher::class); + +$source = "counter += \$value;\n }\n\n"; + + for ($j = 0; $j < METHODS; ++$j) { + $source .= " public function method{$j}(string \$name): string\n {\n"; + $source .= " \$this->items[] = \$name;\n"; + $source .= " return \$this->parentMethod(\$name) . self::NAME . \$this->handle({$j});\n }\n\n"; + } + + $source .= "}\n\n"; +} + +$sourceFilePath = tempnam(sys_get_temp_dir(), 'bench_') . '.php'; +file_put_contents($sourceFilePath, $source); + +$stmts = $testingParser->parseFileToDecoratedNodes($sourceFilePath); + +/** @var Class_[] $classes */ +$classes = array_values(array_filter( + $betterNodeFinder->findInstanceOf($stmts, Class_::class), + static fn (Class_ $class): bool => $class->isFinal() +)); + +// keep the original in-class scope, as RectorRunner passes the scope of the node before the change +$scopes = []; +foreach ($classes as $key => $class) { + $scope = $class->getAttribute(AttributeKey::SCOPE); + if (! $scope instanceof MutatingScope || ! $scope->isInClass()) { + fwrite(STDERR, 'Class node is missing the in-class scope, bench would not hit the refresh path' . PHP_EOL); + exit(1); + } + + $scopes[$key] = $scope; +} + +$cpuSeconds = static function (): float { + $usage = getrusage(); + return $usage['ru_utime.tv_sec'] + $usage['ru_utime.tv_usec'] / 1_000_000 + + $usage['ru_stime.tv_sec'] + $usage['ru_stime.tv_usec'] / 1_000_000; +}; + +$run = static function () use ($changedNodeScopeRefresher, $classes, $scopes, $sourceFilePath, $isChanged): void { + foreach ($classes as $key => $class) { + if ($isChanged) { + if ($class->stmts[0] instanceof ClassConst && $class->stmts[0]->consts[0]->name->toString() === 'EXTRA') { + array_shift($class->stmts); + } else { + // BetterReflection asserts the constant lines, so mirror the class lines as rules do for new nodes + $lines = [ + 'startLine' => $class->getStartLine(), + 'endLine' => $class->getStartLine(), + ]; + array_unshift($class->stmts, new ClassConst([new Const_('EXTRA', new Int_(1), $lines)], 0, $lines)); + } + } + + $changedNodeScopeRefresher->refresh($class, $sourceFilePath, $scopes[$key]); + } +}; + +// warm up autoloading/reflection caches once, excluded from timing +$run(); + +$memoryStart = memory_get_usage(); +$wallStart = microtime(true); +$cpuStart = $cpuSeconds(); + +for ($repeat = 0; $repeat < REPEAT; ++$repeat) { + $run(); +} + +$cpu = $cpuSeconds() - $cpuStart; +$wall = microtime(true) - $wallStart; +$memory = memory_get_usage() - $memoryStart; + +@unlink($sourceFilePath); + +$totalCalls = REPEAT * count($classes); + +printf('mode: %s%s', $isChanged ? 'changed' : 'same', PHP_EOL); +printf('classes: %d, methods/class: %d, repeats: %d, refresh() calls: %d%s', count($classes), METHODS + 1, REPEAT, $totalCalls, PHP_EOL); +printf('CPU: %.4f s (%.3f us/call)%s', $cpu, $cpu / $totalCalls * 1_000_000, PHP_EOL); +printf('wall: %.4f s%s', $wall, PHP_EOL); +printf('memory growth: %.2f MB%s', $memory / 1024 / 1024, PHP_EOL); diff --git a/src/NodeTypeResolver/PHPStan/Scope/AstClassReflectionFactory.php b/src/NodeTypeResolver/PHPStan/Scope/AstClassReflectionFactory.php index 32e57899a2b..19dc1f6a50b 100644 --- a/src/NodeTypeResolver/PHPStan/Scope/AstClassReflectionFactory.php +++ b/src/NodeTypeResolver/PHPStan/Scope/AstClassReflectionFactory.php @@ -19,6 +19,7 @@ use PHPStan\BetterReflection\SourceLocator\Located\LocatedSource; use PHPStan\Reflection\ClassReflection; use PHPStan\Reflection\ClassReflectionFactory; +use Rector\PhpParser\Printer\BetterStandardPrinter; /** * The class reflection from the reflection provider is built from the original file, @@ -26,11 +27,19 @@ * * @see \PHPStan\Analyser\StmtHandler\ClassLikeHandler::createAstClassReflection() */ -final readonly class AstClassReflectionFactory +final class AstClassReflectionFactory { + /** + * Last rebuilt reflection per class, keyed by file and class node, with the hash of the class content it was built from + * + * @var array + */ + private array $classReflectionsByClass = []; + public function __construct( - private Reflector $reflector, - private ClassReflectionFactory $classReflectionFactory, + private readonly Reflector $reflector, + private readonly ClassReflectionFactory $classReflectionFactory, + private readonly BetterStandardPrinter $betterStandardPrinter, ) { } @@ -39,6 +48,15 @@ public function create(Class_|Interface_|Enum_ $classLike, MutatingScope $mutati $filePath = $mutatingScope->getFile(); $namespace = $mutatingScope->getNamespace(); + // the class node is refreshed on every change in it, while its members often stay the same between passes; + // reuse the reflection built from the same content, so its cache key and member caches are not created again + $classKey = $filePath . ':' . $namespace . '\\' . $classLike->name; + $contentHash = hash('xxh128', $this->betterStandardPrinter->print($classLike)); + + if (isset($this->classReflectionsByClass[$classKey]) && $this->classReflectionsByClass[$classKey][0] === $contentHash) { + return $this->classReflectionsByClass[$classKey][1]; + } + $betterReflectionClass = new NodeToReflection()->__invoke( $this->reflector, $classLike, @@ -55,14 +73,18 @@ public function create(Class_|Interface_|Enum_ $classLike, MutatingScope $mutati : new ReflectionClassAdapter($betterReflectionClass); // the member caches are keyed by the class reflection cache key, - // so each rebuilt reflection needs its own key not to read members of the former one - return $this->classReflectionFactory->create( + // so each changed class content needs its own key not to read members of the former one + $classReflection = $this->classReflectionFactory->create( $betterReflectionClass->getName(), $reflectionClassAdapter, null, null, null, - uniqid($filePath . ':', true) + $filePath . ':' . $contentHash ); + + $this->classReflectionsByClass[$classKey] = [$contentHash, $classReflection]; + + return $classReflection; } } diff --git a/tests/Issues/ClassReflectionRefreshInPlace/ClassReflectionRefreshInPlaceTest.php b/tests/Issues/ClassReflectionRefreshInPlace/ClassReflectionRefreshInPlaceTest.php new file mode 100644 index 00000000000..c7707439013 --- /dev/null +++ b/tests/Issues/ClassReflectionRefreshInPlace/ClassReflectionRefreshInPlaceTest.php @@ -0,0 +1,28 @@ +doTestFile($filePath); + } + + public static function provideData(): Iterator + { + return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule.php'; + } +} diff --git a/tests/Issues/ClassReflectionRefreshInPlace/Fixture/add_both_traits_once.php.inc b/tests/Issues/ClassReflectionRefreshInPlace/Fixture/add_both_traits_once.php.inc new file mode 100644 index 00000000000..735947f52dc --- /dev/null +++ b/tests/Issues/ClassReflectionRefreshInPlace/Fixture/add_both_traits_once.php.inc @@ -0,0 +1,27 @@ + +----- + diff --git a/tests/Issues/ClassReflectionRefreshInPlace/Source/AddOtherTraitByReflectionRector.php b/tests/Issues/ClassReflectionRefreshInPlace/Source/AddOtherTraitByReflectionRector.php new file mode 100644 index 00000000000..1bd3e5584e9 --- /dev/null +++ b/tests/Issues/ClassReflectionRefreshInPlace/Source/AddOtherTraitByReflectionRector.php @@ -0,0 +1,54 @@ +> + */ + public function getNodeTypes(): array + { + return [Class_::class]; + } + + /** + * @param Class_ $node + */ + public function refactor(Node $node): ?Class_ + { + $classReflection = $this->reflectionResolver->resolveClassReflection($node); + if (! $classReflection instanceof ClassReflection || $classReflection->hasTraitUse(OtherTrait::class)) { + return null; + } + + $node->stmts = array_merge([new TraitUse([new FullyQualified(OtherTrait::class)])], $node->stmts); + + return $node; + } +} diff --git a/tests/Issues/ClassReflectionRefreshInPlace/Source/OtherTrait.php b/tests/Issues/ClassReflectionRefreshInPlace/Source/OtherTrait.php new file mode 100644 index 00000000000..9ef38bb92c0 --- /dev/null +++ b/tests/Issues/ClassReflectionRefreshInPlace/Source/OtherTrait.php @@ -0,0 +1,9 @@ +withRules([AddTraitByReflectionRector::class, AddOtherTraitByReflectionRector::class]);