Skip to content

Commit a3151d0

Browse files
TomasVotrubaTomasVotruba
authored
[perf] Key config by method/property name in Add*Type rules (#8744)
1 parent 8c738d0 commit a3151d0

4 files changed

Lines changed: 135 additions & 31 deletions

File tree

‎bench/add_property_type_bucket_bench.php‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* Benchmark for AddPropertyTypeDeclarationRector (representative of the Add*Type bucket change).
7+
*
8+
* AddParamTypeDeclarationRector / AddPropertyTypeDeclarationRector / AddReturnTypeDeclarationRector
9+
* each scanned their whole configured list per visited node, running isName()/isStringName() on
10+
* each entry before the class/object-type check. The change buckets the list by lowercased old
11+
* name once in configure(), so a node only visits entries sharing its name.
12+
*
13+
* This bench drives refactor() over many untyped properties whose names are NOT in the map (the
14+
* common case), isolating the scan cost.
15+
*
16+
* Run on both branches to compare:
17+
* php bench/add_property_type_bucket_bench.php
18+
*
19+
* Reports CPU time (user+sys) and wall time; CPU is the reliable metric, wall is noisy under
20+
* concurrent load.
21+
*/
22+
23+
use PhpParser\Node\Stmt\Property;
24+
use PHPStan\Type\StringType;
25+
use Rector\DependencyInjection\LazyContainerFactory;
26+
use Rector\PhpParser\Node\BetterNodeFinder;
27+
use Rector\Testing\TestingParser\TestingParser;
28+
use Rector\TypeDeclaration\Rector\Property\AddPropertyTypeDeclarationRector;
29+
use Rector\TypeDeclaration\ValueObject\AddPropertyTypeDeclaration;
30+
31+
require __DIR__ . '/../vendor/autoload.php';
32+
33+
const ENTRIES = 400; // configured add-property-type entries
34+
const PROPERTIES = 150; // untyped properties per pass, names not in the map
35+
const REPEAT = 200; // outer repeats over the whole set
36+
37+
$rectorConfig = (new LazyContainerFactory())->create();
38+
$rectorConfig->boot();
39+
40+
/** @var TestingParser $testingParser */
41+
$testingParser = $rectorConfig->make(TestingParser::class);
42+
43+
/** @var BetterNodeFinder $betterNodeFinder */
44+
$betterNodeFinder = $rectorConfig->make(BetterNodeFinder::class);
45+
46+
$rector = $rectorConfig->make(AddPropertyTypeDeclarationRector::class);
47+
48+
$addPropertyTypeDeclarations = [];
49+
for ($i = 0; $i < ENTRIES; ++$i) {
50+
$addPropertyTypeDeclarations[] = new AddPropertyTypeDeclaration('Bench\\SomeParent' . $i, 'oldProperty' . $i, new StringType());
51+
}
52+
53+
$rector->configure($addPropertyTypeDeclarations);
54+
55+
$source = "<?php\n\nnamespace Bench;\n\nclass SomeClass\n{\n";
56+
for ($i = 0; $i < PROPERTIES; ++$i) {
57+
// property names deliberately absent from the configured entries
58+
$source .= " public \$freshProperty{$i};\n";
59+
}
60+
61+
$source .= "}\n";
62+
63+
$sourceFilePath = tempnam(sys_get_temp_dir(), 'bench_') . '.php';
64+
file_put_contents($sourceFilePath, $source);
65+
66+
$stmts = $testingParser->parseFileToDecoratedNodes($sourceFilePath);
67+
68+
/** @var Property[] $properties */
69+
$properties = $betterNodeFinder->findInstanceOf($stmts, Property::class);
70+
71+
$cpuSeconds = static function (): float {
72+
$usage = getrusage();
73+
return $usage['ru_utime.tv_sec'] + $usage['ru_utime.tv_usec'] / 1_000_000
74+
+ $usage['ru_stime.tv_sec'] + $usage['ru_stime.tv_usec'] / 1_000_000;
75+
};
76+
77+
$run = static function () use ($rector, $properties): void {
78+
foreach ($properties as $property) {
79+
$rector->refactor($property);
80+
}
81+
};
82+
83+
// warm up autoloading/reflection caches once, excluded from timing
84+
$run();
85+
86+
$wallStart = microtime(true);
87+
$cpuStart = $cpuSeconds();
88+
89+
for ($repeat = 0; $repeat < REPEAT; ++$repeat) {
90+
$run();
91+
}
92+
93+
$cpu = $cpuSeconds() - $cpuStart;
94+
$wall = microtime(true) - $wallStart;
95+
96+
@unlink($sourceFilePath);
97+
98+
$totalCalls = REPEAT * count($properties);
99+
100+
printf('entries: %d, properties: %d, repeats: %d, refactor() calls: %d%s', ENTRIES, count($properties), REPEAT, $totalCalls, PHP_EOL);
101+
printf('CPU: %.4f s (%.3f us/call)%s', $cpu, $cpu / $totalCalls * 1_000_000, PHP_EOL);
102+
printf('wall: %.4f s%s', $wall, PHP_EOL);

‎rules/TypeDeclaration/Rector/ClassMethod/AddParamTypeDeclarationRector.php‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,9 @@
3131
final class AddParamTypeDeclarationRector extends AbstractRector implements ConfigurableRectorInterface
3232
{
3333
/**
34-
* @var AddParamTypeDeclaration[]
34+
* @var array<string, AddParamTypeDeclaration[]>
3535
*/
36-
private array $addParamTypeDeclarations = [];
36+
private array $addParamTypeDeclarationsByMethod = [];
3737

3838
private bool $hasChanged = false;
3939

@@ -103,11 +103,11 @@ public function refactor(Node $node): ?Node
103103
}
104104

105105
$methodName = $this->getName($classMethod);
106-
foreach ($this->addParamTypeDeclarations as $addParamTypeDeclaration) {
107-
if (! $this->nodeNameResolver->isStringName($methodName, $addParamTypeDeclaration->getMethodName())) {
108-
continue;
109-
}
106+
if ($methodName === null) {
107+
continue;
108+
}
110109

110+
foreach ($this->addParamTypeDeclarationsByMethod[strtolower($methodName)] ?? [] as $addParamTypeDeclaration) {
111111
if (! $this->isObjectType($node, $addParamTypeDeclaration->getObjectType())) {
112112
continue;
113113
}
@@ -130,7 +130,10 @@ public function configure(array $configuration): void
130130
{
131131
Assert::allIsAOf($configuration, AddParamTypeDeclaration::class);
132132

133-
$this->addParamTypeDeclarations = $configuration;
133+
$this->addParamTypeDeclarationsByMethod = [];
134+
foreach ($configuration as $addParamTypeDeclaration) {
135+
$this->addParamTypeDeclarationsByMethod[strtolower($addParamTypeDeclaration->getMethodName())][] = $addParamTypeDeclaration;
136+
}
134137
}
135138

136139
private function shouldSkip(Class_|Interface_ $classLike, ClassMethod $classMethod): bool

‎rules/TypeDeclaration/Rector/ClassMethod/AddReturnTypeDeclarationRector.php‎

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@
3333
final class AddReturnTypeDeclarationRector extends AbstractRector implements ConfigurableRectorInterface
3434
{
3535
/**
36-
* @var AddReturnTypeDeclaration[]
36+
* @var array<string, AddReturnTypeDeclaration[]>
3737
*/
38-
private array $methodReturnTypes = [];
38+
private array $methodReturnTypesByMethod = [];
3939

4040
private bool $hasChanged = false;
4141

@@ -94,24 +94,15 @@ public function refactor(Node $node): ?Node
9494
return null;
9595
}
9696

97-
$classMethodNames = [];
9897
foreach ($node->getMethods() as $classMethod) {
99-
$classMethodNames[strtolower((string) $this->getName($classMethod))] = true;
100-
}
101-
102-
foreach ($this->methodReturnTypes as $methodReturnType) {
103-
// cheap name check before the expensive type resolution below
104-
if (! isset($classMethodNames[strtolower($methodReturnType->getMethod())])) {
105-
continue;
106-
}
107-
108-
$objectType = $methodReturnType->getObjectType();
109-
if (! $this->isObjectType($node, $objectType)) {
98+
$methodName = $this->getName($classMethod);
99+
if ($methodName === null) {
110100
continue;
111101
}
112102

113-
foreach ($node->getMethods() as $classMethod) {
114-
if (! $this->isName($classMethod, $methodReturnType->getMethod())) {
103+
foreach ($this->methodReturnTypesByMethod[strtolower($methodName)] ?? [] as $methodReturnType) {
104+
$objectType = $methodReturnType->getObjectType();
105+
if (! $this->isObjectType($node, $objectType)) {
115106
continue;
116107
}
117108

@@ -138,7 +129,10 @@ public function configure(array $configuration): void
138129
{
139130
Assert::allIsAOf($configuration, AddReturnTypeDeclaration::class);
140131

141-
$this->methodReturnTypes = $configuration;
132+
$this->methodReturnTypesByMethod = [];
133+
foreach ($configuration as $methodReturnType) {
134+
$this->methodReturnTypesByMethod[strtolower($methodReturnType->getMethod())][] = $methodReturnType;
135+
}
142136
}
143137

144138
private function processClassMethodNodeWithTypehints(

‎rules/TypeDeclaration/Rector/Property/AddPropertyTypeDeclarationRector.php‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@
2525
final class AddPropertyTypeDeclarationRector extends AbstractRector implements ConfigurableRectorInterface
2626
{
2727
/**
28-
* @var AddPropertyTypeDeclaration[]
28+
* @var array<string, AddPropertyTypeDeclaration[]>
2929
*/
30-
private array $addPropertyTypeDeclarations = [];
30+
private array $addPropertyTypeDeclarationsByProperty = [];
3131

3232
public function __construct(
3333
private readonly StaticTypeMapper $staticTypeMapper
@@ -82,11 +82,12 @@ public function refactor(Node $node): ?Node
8282
return null;
8383
}
8484

85-
foreach ($this->addPropertyTypeDeclarations as $addPropertyTypeDeclaration) {
86-
if (! $this->isName($node, $addPropertyTypeDeclaration->getPropertyName())) {
87-
continue;
88-
}
85+
$propertyName = $this->getName($node);
86+
if ($propertyName === null) {
87+
return null;
88+
}
8989

90+
foreach ($this->addPropertyTypeDeclarationsByProperty[strtolower($propertyName)] ?? [] as $addPropertyTypeDeclaration) {
9091
if (! $this->isClassReflectionType($classReflection, $addPropertyTypeDeclaration->getClass())) {
9192
continue;
9293
}
@@ -113,7 +114,11 @@ public function refactor(Node $node): ?Node
113114
public function configure(array $configuration): void
114115
{
115116
Assert::allIsAOf($configuration, AddPropertyTypeDeclaration::class);
116-
$this->addPropertyTypeDeclarations = $configuration;
117+
118+
$this->addPropertyTypeDeclarationsByProperty = [];
119+
foreach ($configuration as $addPropertyTypeDeclaration) {
120+
$this->addPropertyTypeDeclarationsByProperty[strtolower($addPropertyTypeDeclaration->getPropertyName())][] = $addPropertyTypeDeclaration;
121+
}
117122
}
118123

119124
private function isClassReflectionType(ClassReflection $classReflection, string $type): bool

0 commit comments

Comments
 (0)