Skip to content

Commit 39deace

Browse files
TomasVotrubaTomasVotruba
authored
[perf] Key config by old name in Arguments/Removing rules (#8743)
1 parent f3e63ff commit 39deace

5 files changed

Lines changed: 151 additions & 35 deletions

File tree

‎bench/remove_method_call_param_bucket_bench.php‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* Benchmark for RemoveMethodCallParamRector (representative of the Arguments/Removing bucket change).
7+
*
8+
* RemoveMethodCallParamRector / ArgumentAdderRector / ArgumentRemoverRector / RemoveFuncCallRector
9+
* each scanned their whole configured list per visited node, running isName() on each entry. The
10+
* change buckets the list by lowercased old method name once in configure() (RemoveFuncCallRector
11+
* flips to a lowercased name set), so a node only visits entries sharing its name.
12+
*
13+
* This bench drives refactor() over many method calls whose names are NOT in the map (the common
14+
* case), isolating the scan cost.
15+
*
16+
* Run on both branches to compare:
17+
* php bench/remove_method_call_param_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\Expr\MethodCall;
24+
use Rector\Arguments\Rector\MethodCall\RemoveMethodCallParamRector;
25+
use Rector\Arguments\ValueObject\RemoveMethodCallParam;
26+
use Rector\DependencyInjection\LazyContainerFactory;
27+
use Rector\PhpParser\Node\BetterNodeFinder;
28+
use Rector\Testing\TestingParser\TestingParser;
29+
30+
require __DIR__ . '/../vendor/autoload.php';
31+
32+
const ENTRIES = 400; // configured remove-method-call-param entries
33+
const CALLS = 200; // method calls per pass, names not in the map
34+
const REPEAT = 200; // outer repeats over the whole set
35+
36+
$rectorConfig = (new LazyContainerFactory())->create();
37+
$rectorConfig->boot();
38+
39+
/** @var TestingParser $testingParser */
40+
$testingParser = $rectorConfig->make(TestingParser::class);
41+
42+
/** @var BetterNodeFinder $betterNodeFinder */
43+
$betterNodeFinder = $rectorConfig->make(BetterNodeFinder::class);
44+
45+
$rector = $rectorConfig->make(RemoveMethodCallParamRector::class);
46+
47+
$removeMethodCallParams = [];
48+
for ($i = 0; $i < ENTRIES; ++$i) {
49+
$removeMethodCallParams[] = new RemoveMethodCallParam('Bench\\SomeClass' . $i, 'oldMethod' . $i, 0);
50+
}
51+
52+
$rector->configure($removeMethodCallParams);
53+
54+
$source = "<?php\n\nnamespace Bench;\n\n\$value = new Other();\n";
55+
for ($i = 0; $i < CALLS; ++$i) {
56+
// method names deliberately absent from the configured entries
57+
$source .= "\$value->freshCall{$i}(1, 2);\n";
58+
}
59+
60+
$sourceFilePath = tempnam(sys_get_temp_dir(), 'bench_') . '.php';
61+
file_put_contents($sourceFilePath, $source);
62+
63+
$stmts = $testingParser->parseFileToDecoratedNodes($sourceFilePath);
64+
65+
/** @var MethodCall[] $methodCalls */
66+
$methodCalls = $betterNodeFinder->findInstanceOf($stmts, MethodCall::class);
67+
68+
$cpuSeconds = static function (): float {
69+
$usage = getrusage();
70+
return $usage['ru_utime.tv_sec'] + $usage['ru_utime.tv_usec'] / 1_000_000
71+
+ $usage['ru_stime.tv_sec'] + $usage['ru_stime.tv_usec'] / 1_000_000;
72+
};
73+
74+
$run = static function () use ($rector, $methodCalls): void {
75+
foreach ($methodCalls as $methodCall) {
76+
$rector->refactor($methodCall);
77+
}
78+
};
79+
80+
// warm up autoloading/reflection caches once, excluded from timing
81+
$run();
82+
83+
$wallStart = microtime(true);
84+
$cpuStart = $cpuSeconds();
85+
86+
for ($repeat = 0; $repeat < REPEAT; ++$repeat) {
87+
$run();
88+
}
89+
90+
$cpu = $cpuSeconds() - $cpuStart;
91+
$wall = microtime(true) - $wallStart;
92+
93+
@unlink($sourceFilePath);
94+
95+
$totalCalls = REPEAT * count($methodCalls);
96+
97+
printf('entries: %d, calls: %d, repeats: %d, refactor() calls: %d%s', ENTRIES, count($methodCalls), REPEAT, $totalCalls, PHP_EOL);
98+
printf('CPU: %.4f s (%.3f us/call)%s', $cpu, $cpu / $totalCalls * 1_000_000, PHP_EOL);
99+
printf('wall: %.4f s%s', $wall, PHP_EOL);

‎rules/Arguments/Rector/ClassMethod/ArgumentAdderRector.php‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,9 @@
4040
final class ArgumentAdderRector extends AbstractRector implements ConfigurableRectorInterface
4141
{
4242
/**
43-
* @var ArgumentAdder[]|ArgumentAdderWithoutDefaultValue[]
43+
* @var array<string, array<ArgumentAdder|ArgumentAdderWithoutDefaultValue>>
4444
*/
45-
private array $addedArguments = [];
45+
private array $addedArgumentsByMethod = [];
4646

4747
private bool $hasChanged = false;
4848

@@ -136,7 +136,11 @@ public function refactor(Node $node): MethodCall|StaticCall|Class_|null
136136
public function configure(array $configuration): void
137137
{
138138
Assert::allIsAnyOf($configuration, [ArgumentAdder::class, ArgumentAdderWithoutDefaultValue::class]);
139-
$this->addedArguments = $configuration;
139+
140+
$this->addedArgumentsByMethod = [];
141+
foreach ($configuration as $addedArgument) {
142+
$this->addedArgumentsByMethod[strtolower($addedArgument->getMethod())][] = $addedArgument;
143+
}
140144
}
141145

142146
private function isObjectTypeMatch(MethodCall|StaticCall $call, ObjectType $objectType): bool
@@ -380,11 +384,7 @@ private function refactorCall(StaticCall|MethodCall $call): void
380384
return;
381385
}
382386

383-
foreach ($this->addedArguments as $addedArgument) {
384-
if (! $this->nodeNameResolver->isStringName($callName, $addedArgument->getMethod())) {
385-
continue;
386-
}
387-
387+
foreach ($this->addedArgumentsByMethod[strtolower($callName)] ?? [] as $addedArgument) {
388388
if (! $this->isObjectTypeMatch($call, $addedArgument->getObjectType())) {
389389
continue;
390390
}
@@ -395,11 +395,12 @@ private function refactorCall(StaticCall|MethodCall $call): void
395395

396396
private function refactorClassMethod(Class_ $class, ClassMethod $classMethod): void
397397
{
398-
foreach ($this->addedArguments as $addedArgument) {
399-
if (! $this->isName($classMethod, $addedArgument->getMethod())) {
400-
continue;
401-
}
398+
$methodName = $this->getName($classMethod);
399+
if ($methodName === null) {
400+
return;
401+
}
402402

403+
foreach ($this->addedArgumentsByMethod[strtolower($methodName)] ?? [] as $addedArgument) {
403404
if (! $this->isObjectType($class, $addedArgument->getObjectType())) {
404405
continue;
405406
}

‎rules/Arguments/Rector/MethodCall/RemoveMethodCallParamRector.php‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@
2626
final class RemoveMethodCallParamRector extends AbstractRector implements ConfigurableRectorInterface
2727
{
2828
/**
29-
* @var RemoveMethodCallParam[]
29+
* @var array<string, RemoveMethodCallParam[]>
3030
*/
31-
private array $removeMethodCallParams = [];
31+
private array $removeMethodCallParamsByMethod = [];
3232

3333
public function __construct(
3434
private readonly ReflectionResolver $reflectionResolver,
@@ -84,11 +84,12 @@ public function refactor(Node $node): ?Node
8484
return null;
8585
}
8686

87-
foreach ($this->removeMethodCallParams as $removeMethodCallParam) {
88-
if (! $this->isName($node->name, $removeMethodCallParam->getMethodName())) {
89-
continue;
90-
}
87+
$methodName = $this->getName($node->name);
88+
if ($methodName === null) {
89+
return null;
90+
}
9191

92+
foreach ($this->removeMethodCallParamsByMethod[strtolower($methodName)] ?? [] as $removeMethodCallParam) {
9293
if (! $this->isCallerObjectType($node, $removeMethodCallParam)) {
9394
continue;
9495
}
@@ -147,7 +148,11 @@ public function refactor(Node $node): ?Node
147148
public function configure(array $configuration): void
148149
{
149150
Assert::allIsInstanceOf($configuration, RemoveMethodCallParam::class);
150-
$this->removeMethodCallParams = $configuration;
151+
152+
$this->removeMethodCallParamsByMethod = [];
153+
foreach ($configuration as $removeMethodCallParam) {
154+
$this->removeMethodCallParamsByMethod[strtolower($removeMethodCallParam->getMethodName())][] = $removeMethodCallParam;
155+
}
151156
}
152157

153158
private function isCallerObjectType(StaticCall|MethodCall $call, RemoveMethodCallParam $removeMethodCallParam): bool

‎rules/Removing/Rector/ClassMethod/ArgumentRemoverRector.php‎

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

3232
private bool $hasChanged = false;
3333

@@ -73,11 +73,12 @@ public function refactor(Node $node): MethodCall|StaticCall|ClassMethod|null
7373
{
7474
$this->hasChanged = false;
7575

76-
foreach ($this->removedArguments as $removedArgument) {
77-
if (! $this->isName($node->name, $removedArgument->getMethod())) {
78-
continue;
79-
}
76+
$methodName = $this->getName($node->name);
77+
if ($methodName === null) {
78+
return null;
79+
}
8080

81+
foreach ($this->removedArgumentsByMethod[strtolower($methodName)] ?? [] as $removedArgument) {
8182
if (! $this->nodeTypeResolver->isMethodStaticCallOrClassMethodObjectType(
8283
$node,
8384
$removedArgument->getObjectType()
@@ -101,7 +102,11 @@ public function refactor(Node $node): MethodCall|StaticCall|ClassMethod|null
101102
public function configure(array $configuration): void
102103
{
103104
Assert::allIsAOf($configuration, ArgumentRemover::class);
104-
$this->removedArguments = $configuration;
105+
106+
$this->removedArgumentsByMethod = [];
107+
foreach ($configuration as $removedArgument) {
108+
$this->removedArgumentsByMethod[strtolower($removedArgument->getMethod())][] = $removedArgument;
109+
}
105110
}
106111

107112
private function processPosition(

‎rules/Removing/Rector/FuncCall/RemoveFuncCallRector.php‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,10 @@
2020
final class RemoveFuncCallRector extends AbstractRector implements ConfigurableRectorInterface
2121
{
2222
/**
23-
* @var string[]
23+
* keyed by lowercased function name (function names are case-insensitive)
24+
* @var array<string, true>
2425
*/
25-
private array $removedFunctions = [];
26+
private array $removedFunctionNames = [];
2627

2728
public function getRuleDefinition(): RuleDefinition
2829
{
@@ -60,15 +61,16 @@ public function refactor(Node $node): ?int
6061
return null;
6162
}
6263

63-
foreach ($this->removedFunctions as $removedFunction) {
64-
if (! $this->isName($expr->name, $removedFunction)) {
65-
continue;
66-
}
64+
$funcName = $this->getName($expr->name);
65+
if ($funcName === null) {
66+
return null;
67+
}
6768

68-
return NodeVisitor::REMOVE_NODE;
69+
if (! isset($this->removedFunctionNames[strtolower($funcName)])) {
70+
return null;
6971
}
7072

71-
return null;
73+
return NodeVisitor::REMOVE_NODE;
7274
}
7375

7476
/**
@@ -77,6 +79,10 @@ public function refactor(Node $node): ?int
7779
public function configure(array $configuration): void
7880
{
7981
Assert::allString($configuration);
80-
$this->removedFunctions = $configuration;
82+
83+
$this->removedFunctionNames = [];
84+
foreach ($configuration as $removedFunction) {
85+
$this->removedFunctionNames[strtolower($removedFunction)] = true;
86+
}
8187
}
8288
}

0 commit comments

Comments
 (0)