Skip to content

Commit 40b7c71

Browse files
committed
Fix transitive detection performance
1 parent 86d3bdc commit 40b7c71

File tree

1 file changed

+63
-37
lines changed

1 file changed

+63
-37
lines changed

src/Rule/DeadMethodRule.php

Lines changed: 63 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,16 @@ class DeadMethodRule implements Rule
5050

5151
private bool $reportTransitivelyDeadMethodAsSeparateError;
5252

53+
/**
54+
* @var array<string, array{string, int}> methodKey => [file, line]
55+
*/
56+
private array $deadMethods = [];
57+
58+
/**
59+
* @var array<string, list<string>> caller => callee[]
60+
*/
61+
private array $callGraph = [];
62+
5363
public function __construct(
5464
ClassHierarchy $classHierarchy,
5565
bool $reportTransitivelyDeadMethodAsSeparateError = false
@@ -81,10 +91,6 @@ public function processNode(
8191
$methodCallData = $node->get(MethodCallCollector::class);
8292
$entrypointData = $node->get(EntrypointCollector::class);
8393

84-
/** @var array<string, list<string>> $callGraph caller => callee[] */
85-
$callGraph = [];
86-
$deadMethods = [];
87-
8894
foreach ($methodDeclarationData as $file => $data) {
8995
foreach ($data as $typeData) {
9096
$typeName = $typeData['name'];
@@ -111,7 +117,7 @@ public function processNode(
111117

112118
foreach ($methods as $methodName => $methodData) {
113119
$definition = $this->getMethodKey($typeName, $methodName);
114-
$deadMethods[$definition] = [$file, $methodData['line']];
120+
$this->deadMethods[$definition] = [$file, $methodData['line']];
115121
}
116122
}
117123

@@ -132,7 +138,7 @@ public function processNode(
132138
$isWhite = $call->caller === null || Method::isUnsupported($call->caller->methodName);
133139

134140
foreach ($this->getAlternativeCalleeKeys($call) as $possibleCalleeKey) {
135-
$callGraph[$callerKey][] = $possibleCalleeKey;
141+
$this->callGraph[$callerKey][] = $possibleCalleeKey;
136142

137143
if ($isWhite) {
138144
$whiteCallees[] = $possibleCalleeKey;
@@ -145,11 +151,7 @@ public function processNode(
145151
unset($methodCallData);
146152

147153
foreach ($whiteCallees as $whiteCalleeKey) {
148-
unset($deadMethods[$whiteCalleeKey]);
149-
150-
foreach ($this->getTransitiveCalleeKeys($whiteCalleeKey, $callGraph) as $subCallKey) {
151-
unset($deadMethods[$subCallKey]);
152-
}
154+
$this->markTransitiveCallsWhite($whiteCalleeKey);
153155
}
154156

155157
foreach ($entrypointData as $file => $entrypointsInFile) {
@@ -158,34 +160,32 @@ public function processNode(
158160
$call = Call::fromString($entrypoint);
159161

160162
foreach ($this->getAlternativeCalleeKeys($call) as $methodDefinition) {
161-
unset($deadMethods[$methodDefinition]);
163+
unset($this->deadMethods[$methodDefinition]);
162164
}
163165

164-
foreach ($this->getTransitiveCalleeKeys($call->callee->toString(), $callGraph) as $subCallKey) {
165-
unset($deadMethods[$subCallKey]);
166-
}
166+
$this->markTransitiveCallsWhite($call->callee->toString());
167167
}
168168
}
169169
}
170170

171171
$errors = [];
172172

173173
if ($this->reportTransitivelyDeadMethodAsSeparateError) {
174-
foreach ($deadMethods as $deadMethodKey => [$file, $line]) {
174+
foreach ($this->deadMethods as $deadMethodKey => [$file, $line]) {
175175
$errors[] = $this->buildError($deadMethodKey, [], $file, $line);
176176
}
177177

178178
return $errors;
179179
}
180180

181-
$deadGroups = $this->groupDeadMethods($deadMethods, $callGraph);
181+
$deadGroups = $this->groupDeadMethods();
182182

183183
foreach ($deadGroups as $deadGroupKey => $deadSubgroupKeys) {
184-
[$file, $line] = $deadMethods[$deadGroupKey]; // @phpstan-ignore offsetAccess.notFound
184+
[$file, $line] = $this->deadMethods[$deadGroupKey]; // @phpstan-ignore offsetAccess.notFound
185185
$subGroupMap = [];
186186

187187
foreach ($deadSubgroupKeys as $deadSubgroupKey) {
188-
$subGroupMap[$deadSubgroupKey] = $deadMethods[$deadSubgroupKey]; // @phpstan-ignore offsetAccess.notFound
188+
$subGroupMap[$deadSubgroupKey] = $this->deadMethods[$deadSubgroupKey]; // @phpstan-ignore offsetAccess.notFound
189189
}
190190

191191
$errors[] = $this->buildError($deadGroupKey, $subGroupMap, $file, $line);
@@ -287,55 +287,83 @@ private function getAlternativeCalleeKeys(Call $call): array
287287
}
288288

289289
/**
290-
* @param array<string, list<string>> $callGraph
290+
* @param array<string, null> $visitedKeys
291+
*/
292+
private function markTransitiveCallsWhite(string $callerKey, array $visitedKeys = []): void
293+
{
294+
$visitedKeys = $visitedKeys === [] ? [$callerKey => null] : $visitedKeys;
295+
$calleeKeys = $this->callGraph[$callerKey] ?? [];
296+
297+
unset($this->deadMethods[$callerKey]);
298+
299+
foreach ($calleeKeys as $calleeKey) {
300+
if (array_key_exists($calleeKey, $visitedKeys)) {
301+
continue;
302+
}
303+
304+
if (!isset($this->deadMethods[$calleeKey])) {
305+
continue;
306+
}
307+
308+
$this->markTransitiveCallsWhite($calleeKey, array_merge($visitedKeys, [$calleeKey => null]));
309+
}
310+
}
311+
312+
/**
291313
* @param array<string, null> $visitedKeys
292314
* @return list<string>
293315
*/
294-
private function getTransitiveCalleeKeys(string $callerKey, array $callGraph, array $visitedKeys = []): array
316+
private function getTransitiveDeadCalls(string $callerKey, array $visitedKeys = []): array
295317
{
296-
$result = [];
297318
$visitedKeys = $visitedKeys === [] ? [$callerKey => null] : $visitedKeys;
298-
$calleeKeys = $callGraph[$callerKey] ?? [];
319+
$calleeKeys = $this->callGraph[$callerKey] ?? [];
320+
321+
$result = [];
299322

300323
foreach ($calleeKeys as $calleeKey) {
301324
if (array_key_exists($calleeKey, $visitedKeys)) {
302325
continue;
303326
}
304327

328+
if (!isset($this->deadMethods[$calleeKey])) {
329+
continue;
330+
}
331+
305332
$result[] = $calleeKey;
306-
$result = array_merge($result, $this->getTransitiveCalleeKeys($calleeKey, $callGraph, array_merge($visitedKeys, [$calleeKey => null])));
333+
334+
foreach ($this->getTransitiveDeadCalls($calleeKey, array_merge($visitedKeys, [$calleeKey => null])) as $transitiveDead) {
335+
$result[] = $transitiveDead;
336+
}
307337
}
308338

309339
return $result;
310340
}
311341

312342
/**
313-
* @param array<string, mixed> $deadMethods
314-
* @param array<string, list<string>> $callGraph
315343
* @return array<string, list<string>>
316344
*/
317-
private function groupDeadMethods(array $deadMethods, array $callGraph): array
345+
private function groupDeadMethods(): array
318346
{
319347
$deadGroups = [];
320348

321349
/** @var array<string, true> $deadMethodsWithCaller */
322350
$deadMethodsWithCaller = [];
323351

324-
foreach ($callGraph as $caller => $callees) {
325-
if (!array_key_exists($caller, $deadMethods)) {
352+
foreach ($this->callGraph as $caller => $callees) {
353+
if (!array_key_exists($caller, $this->deadMethods)) {
326354
continue;
327355
}
328356

329357
foreach ($callees as $callee) {
330-
if (array_key_exists($callee, $deadMethods)) {
358+
if (array_key_exists($callee, $this->deadMethods)) {
331359
$deadMethodsWithCaller[$callee] = true;
332360
}
333361
}
334362
}
335363

336364
$methodsGrouped = [];
337365

338-
foreach ($deadMethods as $deadMethodKey => $_) {
366+
foreach ($this->deadMethods as $deadMethodKey => $_) {
339367
if (isset($methodsGrouped[$deadMethodKey])) {
340368
continue;
341369
}
@@ -347,23 +375,21 @@ private function groupDeadMethods(array $deadMethods, array $callGraph): array
347375
$deadGroups[$deadMethodKey] = [];
348376
$methodsGrouped[$deadMethodKey] = true;
349377

350-
foreach ($this->getTransitiveCalleeKeys($deadMethodKey, $callGraph) as $transitiveMethodKey) {
351-
if (!isset($deadMethods[$transitiveMethodKey])) {
352-
continue;
353-
}
378+
$transitiveMethodKeys = $this->getTransitiveDeadCalls($deadMethodKey);
354379

380+
foreach ($transitiveMethodKeys as $transitiveMethodKey) {
355381
$deadGroups[$deadMethodKey][] = $transitiveMethodKey;
356382
$methodsGrouped[$transitiveMethodKey] = true;
357383
}
358384
}
359385

360386
// now only cycles remain, lets pick group representatives based on first occurrence
361-
foreach ($deadMethods as $deadMethodKey => $_) {
387+
foreach ($this->deadMethods as $deadMethodKey => $_) {
362388
if (isset($methodsGrouped[$deadMethodKey])) {
363389
continue;
364390
}
365391

366-
$transitiveDeadMethods = $this->getTransitiveCalleeKeys($deadMethodKey, $callGraph);
392+
$transitiveDeadMethods = $this->getTransitiveDeadCalls($deadMethodKey);
367393

368394
$deadGroups[$deadMethodKey] = []; // TODO provide info to some Tip that those are cycles?
369395
$methodsGrouped[$deadMethodKey] = true;

0 commit comments

Comments
 (0)