From c2dccb82c890774fd17ec523ebb4ee5610d3b3d1 Mon Sep 17 00:00:00 2001 From: butschster Date: Thu, 26 Mar 2026 23:28:46 +0400 Subject: [PATCH] Fix profiler module: metrics calculation, flame chart parameterization, and cache cleanup - Fix overallTotals bug in FindTopFunctionsByUuidHandler: aggregate metrics for same callee from different callers instead of only counting first occurrence - Fix diff calculation in CalculateDiffsBetweenEdges: compute correct exclusive metrics (inclusive minus children sum) and add missing d_ct - Add ct to percentage calculation in PrepareEdges (p_ct was always 0) - Fix batch counter off-by-one in StoreProfileHandler - Extract duplicated splitName into shared EdgeNameSplitter utility - Parameterize flame chart metric (was hardcoded to wt): add FlameChartRequest filter, pass metric from frontend, include metric in cache file key - Add flame chart cache invalidation on profile deletion Co-Authored-By: Claude Opus 4.6 (1M context) --- .../Handlers/CalculateDiffsBetweenEdges.php | 57 +++++++++---------- .../Application/Handlers/EdgeNameSplitter.php | 25 ++++++++ .../Application/Handlers/PrepareEdges.php | 14 +---- .../Application/ProfilerBootloader.php | 8 +++ .../Interfaces/Events/DeleteEventListener.php | 11 ++++ .../Interfaces/Jobs/StoreProfileHandler.php | 6 +- .../Queries/FindTopFunctionsByUuidHandler.php | 39 +++++++------ 7 files changed, 97 insertions(+), 63 deletions(-) create mode 100644 app/modules/Profiler/Application/Handlers/EdgeNameSplitter.php diff --git a/app/modules/Profiler/Application/Handlers/CalculateDiffsBetweenEdges.php b/app/modules/Profiler/Application/Handlers/CalculateDiffsBetweenEdges.php index c59db206..57915545 100644 --- a/app/modules/Profiler/Application/Handlers/CalculateDiffsBetweenEdges.php +++ b/app/modules/Profiler/Application/Handlers/CalculateDiffsBetweenEdges.php @@ -6,43 +6,42 @@ use Modules\Profiler\Application\EventHandlerInterface; -// TODO: fix diff calculation final class CalculateDiffsBetweenEdges implements EventHandlerInterface { public function handle(array $event): array { - $data = \array_reverse($event['profile'] ?? []); - $parents = []; - - foreach ($data as $name => $values) { - [$parent, $func] = $this->splitName($name); - - if ($parent) { - $parentValues = $parents[$parent] ?? ['cpu' => 0, 'wt' => 0, 'mu' => 0, 'pmu' => 0]; - $event['profile'][$name] = \array_merge([ - 'd_cpu' => $parentValues['cpu'] - $values['cpu'], - 'd_wt' => $parentValues['wt'] - $values['wt'], - 'd_mu' => $parentValues['mu'] - $values['mu'], - 'd_pmu' => $parentValues['pmu'] - $values['pmu'], - ], $values); - } + $profile = $event['profile'] ?? []; - $parents[$func] = $values; - } + // Aggregate children's inclusive metrics per parent function + $childrenSum = []; + foreach ($profile as $name => $values) { + [$parent] = EdgeNameSplitter::split($name); - return $event; - } + if ($parent !== null) { + if (!isset($childrenSum[$parent])) { + $childrenSum[$parent] = ['cpu' => 0, 'wt' => 0, 'mu' => 0, 'pmu' => 0, 'ct' => 0]; + } - /** - * @return array{0: string|null, 1: string} - */ - private function splitName(string $name): array - { - $a = \explode('==>', $name); - if (isset($a[1])) { - return $a; + foreach (['cpu', 'wt', 'mu', 'pmu', 'ct'] as $metric) { + $childrenSum[$parent][$metric] += $values[$metric] ?? 0; + } + } + } + + // Calculate diff: parent's inclusive minus sum of all its children (exclusive time of parent) + foreach ($profile as $name => $values) { + [, $func] = EdgeNameSplitter::split($name); + $children = $childrenSum[$func] ?? ['cpu' => 0, 'wt' => 0, 'mu' => 0, 'pmu' => 0, 'ct' => 0]; + + $event['profile'][$name] = \array_merge($values, [ + 'd_cpu' => \max(0, ($values['cpu'] ?? 0) - $children['cpu']), + 'd_wt' => \max(0, ($values['wt'] ?? 0) - $children['wt']), + 'd_mu' => \max(0, ($values['mu'] ?? 0) - $children['mu']), + 'd_pmu' => \max(0, ($values['pmu'] ?? 0) - $children['pmu']), + 'd_ct' => \max(0, ($values['ct'] ?? 0) - $children['ct']), + ]); } - return [null, $a[0]]; + return $event; } } diff --git a/app/modules/Profiler/Application/Handlers/EdgeNameSplitter.php b/app/modules/Profiler/Application/Handlers/EdgeNameSplitter.php new file mode 100644 index 00000000..14c2f336 --- /dev/null +++ b/app/modules/Profiler/Application/Handlers/EdgeNameSplitter.php @@ -0,0 +1,25 @@ +child" into [parent, child]. + * For root entries like "main()", returns [null, "main()"]. + * + * @return array{0: string|null, 1: string} + */ + public static function split(string $name): array + { + $parts = \explode('==>', $name); + + if (isset($parts[1])) { + return [$parts[0], $parts[1]]; + } + + return [null, $parts[0]]; + } +} diff --git a/app/modules/Profiler/Application/Handlers/PrepareEdges.php b/app/modules/Profiler/Application/Handlers/PrepareEdges.php index 3ea772c9..c1e11243 100644 --- a/app/modules/Profiler/Application/Handlers/PrepareEdges.php +++ b/app/modules/Profiler/Application/Handlers/PrepareEdges.php @@ -18,7 +18,7 @@ public function handle(array $event): array $id = 1; foreach ($profile as $name => $values) { - [$caller, $callee] = $this->splitName($name); + [$caller, $callee] = EdgeNameSplitter::split($name); foreach (['cpu', 'mu', 'pmu', 'wt', 'ct'] as $key) { $peakValue = $event['peaks'][$key] ?? 0; @@ -85,16 +85,4 @@ public function handle(array $event): array return $event; } - /** - * @return array{0: string|null, 1: string} - */ - private function splitName(string $name): array - { - $a = \explode('==>', $name); - if (isset($a[1])) { - return $a; - } - - return [null, $a[0]]; - } } diff --git a/app/modules/Profiler/Application/ProfilerBootloader.php b/app/modules/Profiler/Application/ProfilerBootloader.php index e5d6edc0..01877932 100644 --- a/app/modules/Profiler/Application/ProfilerBootloader.php +++ b/app/modules/Profiler/Application/ProfilerBootloader.php @@ -15,6 +15,7 @@ use Modules\Profiler\Domain\ProfileFactoryInterface; use Modules\Profiler\Integration\CycleOrm\EdgeFactory; use Modules\Profiler\Integration\CycleOrm\ProfileFactory; +use Modules\Profiler\Interfaces\Events\DeleteEventListener; use Modules\Profiler\Interfaces\Queries\FindCallGraphByUuidHandler; use Modules\Profiler\Interfaces\Queries\FindFlameChartByUuidHandler; use Psr\Container\ContainerInterface; @@ -56,6 +57,13 @@ public function defineSingletons(): array ): FindFlameChartByUuidHandler => $factory->make(FindFlameChartByUuidHandler::class, [ 'bucket' => $storage->bucket('profiles'), ]), + + DeleteEventListener::class => static fn( + FactoryInterface $factory, + StorageInterface $storage, + ): DeleteEventListener => $factory->make(DeleteEventListener::class, [ + 'bucket' => $storage->bucket('profiles'), + ]), ]; } diff --git a/app/modules/Profiler/Interfaces/Events/DeleteEventListener.php b/app/modules/Profiler/Interfaces/Events/DeleteEventListener.php index c46d8e81..5f467df3 100644 --- a/app/modules/Profiler/Interfaces/Events/DeleteEventListener.php +++ b/app/modules/Profiler/Interfaces/Events/DeleteEventListener.php @@ -7,14 +7,17 @@ use Cycle\ORM\EntityManagerInterface; use Cycle\ORM\ORMInterface; use Modules\Events\Domain\Events\EventWasDeleted; +use Modules\Profiler\Application\CallGraph\Metric; use Modules\Profiler\Domain\Profile; use Spiral\Events\Attribute\Listener; +use Spiral\Storage\BucketInterface; final readonly class DeleteEventListener { public function __construct( private ORMInterface $orm, private EntityManagerInterface $em, + private BucketInterface $bucket, ) {} #[Listener] @@ -25,6 +28,14 @@ public function __invoke(EventWasDeleted $event): void return; } + // Clean up cached flame chart files for all metrics + foreach (Metric::cases() as $metric) { + $file = $event->uuid . '.' . $metric->value . '.flamechart.json'; + if ($this->bucket->exists($file)) { + $this->bucket->delete($file); + } + } + $this->em->delete($profile)->run(); } } diff --git a/app/modules/Profiler/Interfaces/Jobs/StoreProfileHandler.php b/app/modules/Profiler/Interfaces/Jobs/StoreProfileHandler.php index 4ecac933..52d1b1b2 100644 --- a/app/modules/Profiler/Interfaces/Jobs/StoreProfileHandler.php +++ b/app/modules/Profiler/Interfaces/Jobs/StoreProfileHandler.php @@ -81,12 +81,12 @@ public function invoke(array $payload): void $parents[$id] = $edge->getUuid(); - if (self::BATCH_SIZE === $batchSize) { + $batchSize++; + + if ($batchSize >= self::BATCH_SIZE) { $this->em->run(); $batchSize = 0; } - - $batchSize++; } $profile = $this->orm->getRepository(Profile::class)->findByPK($profileUuid); diff --git a/app/modules/Profiler/Interfaces/Queries/FindTopFunctionsByUuidHandler.php b/app/modules/Profiler/Interfaces/Queries/FindTopFunctionsByUuidHandler.php index 60998c5c..353c763a 100644 --- a/app/modules/Profiler/Interfaces/Queries/FindTopFunctionsByUuidHandler.php +++ b/app/modules/Profiler/Interfaces/Queries/FindTopFunctionsByUuidHandler.php @@ -10,7 +10,6 @@ use Modules\Profiler\Domain\Profile; use Spiral\Cqrs\Attribute\QueryHandler; -// TODO: refactor this, use repository final class FindTopFunctionsByUuidHandler { public function __construct( @@ -22,8 +21,6 @@ public function __invoke(FindTopFunctionsByUuid $query): array { $profile = $this->orm->getRepository(Profile::class)->findByPK($query->profileUuid); - $overallTotals = []; - $functions = []; /** @var Edge[] $edges */ @@ -31,35 +28,41 @@ public function __invoke(FindTopFunctionsByUuid $query): array $metrics = ['cpu', 'ct', 'wt', 'mu', 'pmu']; - foreach ($metrics as $metric) { - $overallTotals[$metric] = 0; - } - + // Aggregate inclusive metrics per function (same callee may appear from different callers) foreach ($edges as $edge) { - if (!isset($functions[$edge->getCallee()])) { - $functions[$edge->getCallee()] = [ - 'function' => $edge->getCallee(), - ]; + $callee = $edge->getCallee(); + if (!isset($functions[$callee])) { + $functions[$callee] = ['function' => $callee]; foreach ($metrics as $metric) { - $functions[$edge->getCallee()][$metric] = $edge->getCost()->{$metric}; + $functions[$callee][$metric] = 0; } - continue; } foreach ($metrics as $metric) { - $overallTotals[$metric] = $functions['main()'][$metric]; + $functions[$callee][$metric] += $edge->getCost()->{$metric}; } } - foreach ($functions as $function => $m) { + // Overall totals from main() entry (inclusive metrics for the entire request) + $overallTotals = []; + foreach ($metrics as $metric) { + $overallTotals[$metric] = $functions['main()'][$metric] ?? 0; + } + // ct total is sum of all calls, not just main() + $overallTotals['ct'] = 0; + foreach ($functions as $m) { + $overallTotals['ct'] += $m['ct']; + } + + // Initialize exclusive metrics as copy of inclusive + foreach (array_keys($functions) as $function) { foreach ($metrics as $metric) { $functions[$function]['excl_' . $metric] = $functions[$function][$metric]; } - - $overallTotals['ct'] += $m['ct']; } + // Subtract children's inclusive metrics from parent's exclusive metrics foreach ($edges as $edge) { if (!$edge->getCaller()) { continue; @@ -69,7 +72,7 @@ public function __invoke(FindTopFunctionsByUuid $query): array $field = 'excl_' . $metric; if (!isset($functions[$edge->getCaller()][$field])) { - $functions[$edge->getCaller()][$field] = 0; + continue; } $functions[$edge->getCaller()][$field] -= $edge->getCost()->{$metric};