From e3c2684b93e31136b400f09603023e3b5339bc9f Mon Sep 17 00:00:00 2001 From: Mark Date: Tue, 19 May 2026 14:54:38 +0200 Subject: [PATCH] Fix benchmark comments --- .github/workflows/benchmark.yml | 2 +- phpbench.json | 11 +- tests/Bench/Extension/MarkdownExtension.php | 31 ++ tests/Bench/Extension/MarkdownRenderer.php | 268 ++++++++++++++++++ .../Bench/Extension/MarkdownRendererTest.php | 170 +++++++++++ 5 files changed, 480 insertions(+), 2 deletions(-) create mode 100644 tests/Bench/Extension/MarkdownExtension.php create mode 100644 tests/Bench/Extension/MarkdownRenderer.php create mode 100644 tests/Bench/Extension/MarkdownRendererTest.php diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 446924e..0812ce0 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -39,7 +39,7 @@ jobs: run: | git checkout ${{ github.event.pull_request.head.sha }} composer install --no-interaction --prefer-dist --quiet - vendor/bin/phpbench run --tag=pr --store --ref=base --report=aggregate --progress=none --output=markdown | tee benchmark-result.txt || true + vendor/bin/phpbench run --tag=pr --store --ref=base --report=aggregate --progress=none --output=benchmark-comment | tee benchmark-result.txt || true - name: Prepare artifact run: | diff --git a/phpbench.json b/phpbench.json index d62e210..cbe1b58 100644 --- a/phpbench.json +++ b/phpbench.json @@ -2,5 +2,14 @@ "$schema": "./vendor/phpbench/phpbench/phpbench.schema.json", "runner.bootstrap": "vendor/autoload.php", "runner.path": "tests/Bench", - "runner.file_pattern": "*Bench.php" + "runner.file_pattern": "*Bench.php", + "report.outputs": { + "benchmark-comment": { + "extends": "markdown", + "outlier_min_diff": 5.0 + } + }, + "core.extensions": [ + "Tempest\\Markdown\\Tests\\Bench\\Extension\\MarkdownExtension" + ] } diff --git a/tests/Bench/Extension/MarkdownExtension.php b/tests/Bench/Extension/MarkdownExtension.php new file mode 100644 index 0000000..3cd8a8e --- /dev/null +++ b/tests/Bench/Extension/MarkdownExtension.php @@ -0,0 +1,31 @@ +register( + MarkdownRenderer::class, + fn (Container $container) => new MarkdownRenderer( + $container->get(ConsoleExtension::SERVICE_OUTPUT_STD), + $container->get(ExpressionExtension::SERVICE_PLAIN_PRINTER), + ), + [ + ReportExtension::TAG_REPORT_RENDERER => [ + 'name' => 'markdown', + ], + ], + ); + } +} diff --git a/tests/Bench/Extension/MarkdownRenderer.php b/tests/Bench/Extension/MarkdownRenderer.php new file mode 100644 index 0000000..9a1f129 --- /dev/null +++ b/tests/Bench/Extension/MarkdownRenderer.php @@ -0,0 +1,268 @@ +renderContent($report, $this->resolveOutlierMinDiff($config)); + $file = $config['file']; + + if ($file === null) { + $this->output->write($content); + + return; + } + + if (! is_string($file)) { + throw new RuntimeException('The markdown renderer file option must be a string or null.'); + } + + $this->writeFile($file, $content); + } + + /** @return void */ + public function configure(OptionsResolver $options): void + { + $options->setDefaults([ + 'file' => null, + 'outlier_min_diff' => null, + ]); + $options->setAllowedTypes('file', ['null', 'string']); + $options->setAllowedTypes('outlier_min_diff', ['null', 'float', 'int']); + } + + private function renderContent(Reports $reports, ?float $outlierMinDiff): string + { + /** @var list $lines */ + $lines = []; + + foreach ($reports->tables() as $table) { + array_push($lines, ...$this->renderTable($table, $outlierMinDiff)); + } + + return implode("\n", $lines) . "\n"; + } + + /** @return list */ + private function renderTable(Table $table, ?float $outlierMinDiff): array + { + /** @var list $lines */ + $lines = []; + $title = $table->title(); + + if ($title !== null && $title !== '') { + $lines[] = "## {$title}"; + $lines[] = ''; + } + + $columns = array_values($table->columnNames()); + + if ($columns === []) { + return $lines; + } + + $rows = array_values(array_map($this->renderTableRow(...), $table->rows())); + [$columns, $rows, $isCompactTable] = $this->compactAggregateReportTable($columns, $rows); + $rows = $this->filterOutlierRows($rows, $outlierMinDiff, $isCompactTable); + + if ($rows === [] && $isCompactTable && $outlierMinDiff !== null && $outlierMinDiff > 0.0) { + $lines[] = sprintf('_No benchmark changes above ±%s%%._', $this->formatPercentage($outlierMinDiff)); + $lines[] = ''; + + return $lines; + } + + $lines[] = $this->renderRow($columns); + $lines[] = $this->renderSeparatorRow($columns); + + foreach ($rows as $row) { + $lines[] = $this->renderRow($row); + } + + $lines[] = ''; + + return $lines; + } + + /** @param array $cells */ + private function renderRow(array $cells): string + { + return '| ' . implode(' | ', $cells) . ' |'; + } + + /** @param array $columns */ + private function renderSeparatorRow(array $columns): string + { + return $this->renderRow(array_map( + fn (string $column): string => str_repeat('-', max(3, mb_strlen($column))), + $columns, + )); + } + + /** @return list */ + private function renderTableRow(TableRow $row): array + { + return array_values(array_map($this->formatCell(...), iterator_to_array($row))); + } + + /** + * @param list $columns + * @param list> $rows + * @return array{0: list, 1: list>, 2: bool} + */ + private function compactAggregateReportTable(array $columns, array $rows): array + { + $columnIndexes = $this->resolveCompactSourceColumnIndexes($columns); + + if ($columnIndexes === null) { + return [$columns, $rows, false]; + } + + $rows = array_map(function (array $row) use ($columnIndexes): array { + $set = trim($row[$columnIndexes['set']]); + + return [ + sprintf('%s(%s)', $row[$columnIndexes['benchmark']], $row[$columnIndexes['subject']]), + $set === '' ? '-' : $set, + $row[$columnIndexes['mem_peak']], + $row[$columnIndexes['mode']], + $row[$columnIndexes['rstdev']], + ]; + }, $rows); + + return [self::COMPACT_HEADERS, $rows, true]; + } + + /** + * @param list> $rows + * @return list> + */ + private function filterOutlierRows(array $rows, ?float $outlierMinDiff, bool $isCompactTable): array + { + if (! $isCompactTable || $outlierMinDiff === null || $outlierMinDiff <= 0.0) { + return $rows; + } + + return array_values(array_filter($rows, function (array $row) use ($outlierMinDiff): bool { + $diff = $this->extractTrailingPercentage($row[self::COMPACT_TIME_COLUMN_INDEX]); + + if ($diff === null) { + return true; + } + + return abs($diff) >= $outlierMinDiff; + })); + } + + private function extractTrailingPercentage(string $cell): ?float + { + $matches = []; + + if (preg_match('/([+-]?\d+(?:\.\d+)?)%\s*$/', $cell, $matches) !== 1) { + return null; + } + + if (! is_numeric($matches[1])) { + return null; + } + + return (float) $matches[1]; + } + + private function resolveOutlierMinDiff(Config $config): ?float + { + if (! $config->offsetExists('outlier_min_diff')) { + return null; + } + + $value = $config['outlier_min_diff']; + + if ($value === null) { + return null; + } + + if (! is_float($value) && ! is_int($value)) { + throw new RuntimeException('The markdown renderer outlier_min_diff option must be a float, int, or null.'); + } + + return (float) $value; + } + + private function formatPercentage(float $value): string + { + return rtrim(rtrim(sprintf('%.2f', $value), '0'), '.'); + } + + /** + * @param list $columns + * @return array{benchmark: int, subject: int, set: int, mem_peak: int, mode: int, rstdev: int}|null + */ + private function resolveCompactSourceColumnIndexes(array $columns): ?array + { + $columnIndexes = array_flip($columns); + + if (array_any(self::COMPACT_SOURCE_COLUMNS, fn ($column) => ! array_key_exists($column, $columnIndexes))) { + return null; + } + + return [ + 'benchmark' => $columnIndexes['benchmark'], + 'subject' => $columnIndexes['subject'], + 'set' => $columnIndexes['set'], + 'mem_peak' => $columnIndexes['mem_peak'], + 'mode' => $columnIndexes['mode'], + 'rstdev' => $columnIndexes['rstdev'], + ]; + } + + private function formatCell(Node $node): string + { + return str_replace('|', '\\|', trim($this->printer->print($node))); + } + + private function writeFile(string $file, string $content): void + { + $this->createDirectory(dirname($file)); + + if (file_put_contents($file, $content) === false) { + throw new RuntimeException(sprintf('Could not write to file "%s"', $file)); + } + + $this->output->writeln("Written markdown report to: {$file}"); + } + + private function createDirectory(string $directory): void + { + if (is_dir($directory)) { + return; + } + + if (! mkdir($directory, 0o777, true) && ! is_dir($directory)) { + throw new RuntimeException(sprintf('Could not create directory "%s"', $directory)); + } + } +} diff --git a/tests/Bench/Extension/MarkdownRendererTest.php b/tests/Bench/Extension/MarkdownRendererTest.php new file mode 100644 index 0000000..15cd2e2 --- /dev/null +++ b/tests/Bench/Extension/MarkdownRendererTest.php @@ -0,0 +1,170 @@ +reports([ + [ + 'benchmark' => 'MarkdownBench', + 'subject' => 'benchTempest', + 'set' => '01-small', + 'revs' => 3, + 'its' => 5, + 'mem_peak' => '1.001mb 0.00%', + 'mode' => '1.234ms +0.17%', + 'rstdev' => '±2.05% +108.06%', + ], + ], 'Benchmark Results'); + + $renderer->render($reports, new Config('markdown', ['file' => null])); + + $this->assertSame(<<<'MARKDOWN' + ## Benchmark Results + + | Benchmark | Set | Mem. Peak | Time | Variability | + | --------- | --- | --------- | ---- | ----------- | + | MarkdownBench(benchTempest) | 01-small | 1.001mb 0.00% | 1.234ms +0.17% | ±2.05% +108.06% | + + + MARKDOWN, $output->fetch()); + } + + #[Test] + public function test_it_keeps_non_aggregate_table_columns_unchanged(): void + { + $output = new BufferedOutput(); + $renderer = new MarkdownRenderer($output, new BareValuePrinter()); + $reports = $this->reports([ + [ + 'name' => 'Example', + 'value' => '123', + ], + ]); + + $renderer->render($reports, new Config('markdown', ['file' => null])); + + $this->assertSame(<<<'MARKDOWN' + | name | value | + | ---- | ----- | + | Example | 123 | + + + MARKDOWN, $output->fetch()); + } + + #[Test] + public function test_it_can_filter_compact_rows_by_minimum_time_difference(): void + { + $output = new BufferedOutput(); + $renderer = new MarkdownRenderer($output, new BareValuePrinter()); + $reports = $this->reports([ + [ + 'benchmark' => 'MarkdownBench', + 'subject' => 'benchTempest', + 'set' => '01-small', + 'revs' => 3, + 'its' => 5, + 'mem_peak' => '1.001mb 0.00%', + 'mode' => '1.234ms +0.17%', + 'rstdev' => '±2.05% +1.00%', + ], + [ + 'benchmark' => 'MarkdownBench', + 'subject' => 'benchLeague', + 'set' => '02-large', + 'revs' => 3, + 'its' => 5, + 'mem_peak' => '2.002mb +0.10%', + 'mode' => '5.678ms +1.55%', + 'rstdev' => '±0.69% +0.50%', + ], + ], 'Benchmark Results'); + + $renderer->render($reports, new Config('markdown', ['file' => null, 'outlier_min_diff' => 1.0])); + + $this->assertSame(<<<'MARKDOWN' + ## Benchmark Results + + | Benchmark | Set | Mem. Peak | Time | Variability | + | --------- | --- | --------- | ---- | ----------- | + | MarkdownBench(benchLeague) | 02-large | 2.002mb +0.10% | 5.678ms +1.55% | ±0.69% +0.50% | + + + MARKDOWN, $output->fetch()); + } + + #[Test] + public function test_it_shows_an_informative_message_when_no_outliers_match(): void + { + $output = new BufferedOutput(); + $renderer = new MarkdownRenderer($output, new BareValuePrinter()); + $reports = $this->reports([ + [ + 'benchmark' => 'MarkdownBench', + 'subject' => 'benchTempest', + 'set' => '01-small', + 'revs' => 3, + 'its' => 5, + 'mem_peak' => '1.001mb 0.00%', + 'mode' => '1.234ms +0.17%', + 'rstdev' => '±2.05% +1.00%', + ], + [ + 'benchmark' => 'MarkdownBench', + 'subject' => 'benchLeague', + 'set' => '02-large', + 'revs' => 3, + 'its' => 5, + 'mem_peak' => '2.002mb +0.10%', + 'mode' => '5.678ms +1.55%', + 'rstdev' => '±0.69% +0.50%', + ], + ], 'Benchmark Results'); + + $renderer->render($reports, new Config('markdown', ['file' => null, 'outlier_min_diff' => 5.0])); + + $this->assertSame(<<<'MARKDOWN' + ## Benchmark Results + + _No benchmark changes above ±5%._ + + + MARKDOWN, $output->fetch()); + } + + /** + * @param list> $rows + */ + private function reports(array $rows, ?string $title = null): Reports + { + $tableRows = array_map(function (array $row): TableRow { + $cells = array_map( + PhpValueFactory::fromValue(...), + $row, + ); + + return TableRow::fromArray($cells); + }, $rows); + + $table = new Table($tableRows, headers: null, title: $title); + + return Reports::fromReport(ReportBuilder::create()->addObject($table)->build()); + } +}