Compare PR benchmarks by instruction count

.github/workflows/benchmarks.yml

@@ -2,53 +2,64 @@ name: Benchmarks
 
 on:
   pull_request:
+    paths:
+      - .github/workflows/benchmarks.yml
+      - benchmarks/**
+      - composer.json
+      - src/**
   push:
     branches:
       - master
   workflow_dispatch:
 
 jobs:
   benchmark-pr:
-    name: Run benchmarks
+    name: Count benchmark instructions
     if: github.event_name == 'pull_request'
     runs-on: ubuntu-latest
+    timeout-minutes: 30
     permissions:
       contents: read
-      pull-requests: write
 
     steps:
       - uses: actions/checkout@v7
+        with:
+          path: head
+          fetch-depth: 2
+
+      - id: base-sha
+        run: echo "sha=$(git -C head rev-parse HEAD^1)" >> "$GITHUB_OUTPUT"
+
+      - uses: actions/checkout@v7
+        with:
+          ref: ${{ steps.base-sha.outputs.sha }}
+          path: base
 
       - uses: shivammathur/setup-php@v2
         with:
           coverage: none
           php-version: "8.3"
 
       - uses: ramsey/composer-install@v4
+        with:
+          working-directory: base
 
-      - name: Run PHPBench
-        run: |
-          php -d zend.assertions=-1 vendor/bin/phpbench run \
-            --progress=none \
-            --dump-file=benchmark.xml
+      - uses: ramsey/composer-install@v4
+        with:
+          working-directory: head
 
-      - name: Convert results
-        run: php benchmarks/convert-results.php benchmark.xml > benchmark-results.json
+      - run: sudo apt-get install --yes valgrind
 
-      - uses: benchmark-action/github-action-benchmark@v1
-        if: github.repository == 'webonyx/graphql-php'
-        with:
-          tool: customSmallerIsBetter
-          output-file-path: benchmark-results.json
-          github-token: ${{ secrets.GITHUB_TOKEN }}
-          auto-push: false
-          gh-pages-branch: benchmark-results
-          benchmark-data-dir-path: dev/bench
-          alert-threshold: '150%'
-          # TODO: switch to true once baseline variance is understood
-          fail-on-alert: false
-          comment-on-alert: true
-          comment-always: false
+      - name: Count instructions on base and head
+        run: |
+          php head/benchmarks/count-instructions.php base > base.json &
+          base_pid=$!
+          php head/benchmarks/count-instructions.php head > head.json
+          wait "$base_pid"
+
+      - name: Compare results
+        shell: bash
+        run: php head/benchmarks/compare-results.php base.json head.json | tee --append "$GITHUB_STEP_SUMMARY"
 
   benchmark-history:
     name: Track benchmark history

.github/workflows/test.yml

@@ -86,6 +86,7 @@ jobs:
   benchmarks:
     name: Benchmarks
     runs-on: ubuntu-latest
+    timeout-minutes: 20
 
     steps:
       - name: Checkout

CONTRIBUTING.md

@@ -151,6 +151,21 @@ Check performance:
 composer bench
 ```
 
+On pull requests, CI counts each benchmark's instructions per revolution with [Valgrind](https://valgrind.org).
+The check fails when any benchmark needs more than 1.1x the instructions of the base branch.
+Of the PHPBench annotations, the counter supports only `@BeforeMethods`.
+
+To reproduce the check, install Valgrind.
+Check out the base branch into a second directory.
+Run `composer install` in that directory.
+Then run:
+
+```sh
+php benchmarks/count-instructions.php ../graphql-php-base > base.json
+php benchmarks/count-instructions.php . > head.json
+php benchmarks/compare-results.php base.json head.json
+```
+
 ## Documentation
 
 We document this library by rendering the Markdown files in [docs](docs) with [MkDocs](https://www.mkdocs.org).

benchmarks/compare-results.php

@@ -0,0 +1,48 @@
+<?php declare(strict_types=1);
+
+$baseFile = $argv[1] ?? null;
+$headFile = $argv[2] ?? null;
+if ($baseFile === null || $headFile === null) {
+    fwrite(STDERR, "Usage: php benchmarks/compare-results.php <base.json> <head.json>\n");
+    exit(1);
+}
+
+$threshold = 1.1;
+
+/** @var array<string, int> $base */
+$base = json_decode((string) file_get_contents($baseFile), true, 2, JSON_THROW_ON_ERROR);
+/** @var array<string, int> $head */
+$head = json_decode((string) file_get_contents($headFile), true, 2, JSON_THROW_ON_ERROR);
+
+$ratios = [];
+foreach (array_intersect_key($head, $base) as $comparedName => $comparedHeadInstructions) {
+    $ratios[$comparedName] = $comparedHeadInstructions / $base[$comparedName];
+}
+if ($ratios === []) {
+    throw new RuntimeException("Found no benchmark in both {$baseFile} and {$headFile}.");
+}
+arsort($ratios);
+
+$regressions = array_filter($ratios, static fn (float $ratio): bool => $ratio > $threshold);
+
+$regressionCount = count($regressions);
+$comparedCount = count($ratios);
+echo $regressions === []
+    ? "### No benchmark needs more than {$threshold}x the instructions of base\n\n"
+    : "### {$regressionCount} of {$comparedCount} benchmarks need more than {$threshold}x the instructions of base\n\n";
+echo "Instructions per revolution, counted with Valgrind.\n";
+echo "When this PR edits a benchmark, that benchmark's ratio compares two different workloads.\n\n";
+echo "| Benchmark | Base | Head | Ratio |\n";
+echo "| --- | ---: | ---: | ---: |\n";
+foreach ($ratios as $name => $ratio) {
+    $marker = isset($regressions[$name]) ? ' :warning:' : '';
+    printf("| %s | %s | %s | %.4f%s |\n", $name, number_format($base[$name]), number_format($head[$name]), $ratio, $marker);
+}
+foreach (array_diff_key($head, $base) as $newName => $newInstructions) {
+    printf("| %s | | %s | new |\n", $newName, number_format($newInstructions));
+}
+foreach (array_diff_key($base, $head) as $removedName => $removedInstructions) {
+    printf("| %s | %s | | removed |\n", $removedName, number_format($removedInstructions));
+}
+
+exit($regressions === [] ? 0 : 1);

benchmarks/count-instructions.php

@@ -0,0 +1,100 @@
+<?php declare(strict_types=1);
+
+$checkout = $argv[1] ?? null;
+if ($checkout === null) {
+    fwrite(STDERR, "Usage: php benchmarks/count-instructions.php <checkout>\n");
+    exit(1);
+}
+
+require_once "{$checkout}/vendor/autoload.php";
+
+// Extra revs add up to this many instructions, so the measured work outweighs the noise of setup.
+const INSTRUCTION_BUDGET = 20_000_000;
+
+/**
+ * @param list<string> $command
+ *
+ * @return string The standard error output
+ */
+function runCommand(array $command): string
+{
+    $process = proc_open($command, [1 => ['file', '/dev/null', 'w'], 2 => ['pipe', 'w']], $pipes);
+    $commandLine = implode(' ', $command);
+    if (! is_resource($process)) {
+        throw new RuntimeException("Failed to start: {$commandLine}.");
+    }
+
+    $stderr = (string) stream_get_contents($pipes[2]);
+    $exitCode = proc_close($process);
+    if ($exitCode !== 0) {
+        throw new RuntimeException("Exit code {$exitCode} from {$commandLine}:\n{$stderr}");
+    }
+
+    return $stderr;
+}
+
+function countInstructions(string $checkout, string $subject, int $revs): int
+{
+    $benchmarksDirectory = __DIR__;
+    $stderr = runCommand([
+        'valgrind',
+        '--tool=cachegrind',
+        '--cachegrind-out-file=/dev/null',
+        PHP_BINARY,
+        '-d',
+        'zend.assertions=-1',
+        '-d',
+        'opcache.enable_cli=0',
+        "{$benchmarksDirectory}/run-subject.php",
+        $checkout,
+        $subject,
+        (string) $revs,
+    ]);
+    if (preg_match('/I\s+refs:\s+([\d,]+)/', $stderr, $match) !== 1) {
+        throw new RuntimeException("No instruction count for {$subject}:\n{$stderr}");
+    }
+
+    $instructions = str_replace(',', '', $match[1]);
+
+    return (int) $instructions;
+}
+
+$files = glob("{$checkout}/benchmarks/*Bench.php");
+if ($files === false) {
+    throw new RuntimeException("Failed to list benchmarks in {$checkout}.");
+}
+
+$instructionsPerRev = [];
+foreach ($files as $file) {
+    $shortClassName = basename($file, '.php');
+    $className = "GraphQL\\Benchmarks\\{$shortClassName}";
+    if (! class_exists($className)) {
+        throw new RuntimeException("Failed to load benchmark class: {$className}.");
+    }
+
+    $class = new ReflectionClass($className);
+    foreach ($class->getMethods(ReflectionMethod::IS_PUBLIC) as $method) {
+        if (strpos($method->getName(), 'bench') !== 0) {
+            continue;
+        }
+
+        $subject = "{$class->getShortName()}::{$method->getName()}";
+        // The 1-rev run holds bootstrap, setup and first-run costs, so the difference cancels them.
+        $baselineInstructions = countInstructions($checkout, $subject, 1);
+        $probeInstructions = countInstructions($checkout, $subject, 2);
+        $probeRevInstructions = max(1, $probeInstructions - $baselineInstructions);
+        $extraRevs = (int) ceil(INSTRUCTION_BUDGET / $probeRevInstructions);
+        $measuredInstructions = $extraRevs === 1
+            ? $probeInstructions
+            : countInstructions($checkout, $subject, 1 + $extraRevs);
+        if ($measuredInstructions <= $baselineInstructions) {
+            throw new RuntimeException("Expected more instructions with more revs: {$subject}.");
+        }
+
+        $instructionsPerRev[$subject] = intdiv($measuredInstructions - $baselineInstructions, $extraRevs);
+        fwrite(STDERR, "{$subject}: {$instructionsPerRev[$subject]}\n");
+    }
+}
+
+$json = json_encode($instructionsPerRev, JSON_PRETTY_PRINT | JSON_THROW_ON_ERROR);
+echo "{$json}\n";

benchmarks/run-subject.php

@@ -0,0 +1,39 @@
+<?php declare(strict_types=1);
+
+// Runs a PHPBench subject without PHPBench, so count-instructions.php can measure it under Valgrind.
+$checkout = $argv[1] ?? null;
+$subject = $argv[2] ?? null;
+$revs = $argv[3] ?? null;
+if ($checkout === null || $subject === null || $revs === null) {
+    fwrite(STDERR, "Usage: php benchmarks/run-subject.php <checkout> <Class::method> <revs>\n");
+    exit(1);
+}
+
+require_once "{$checkout}/vendor/autoload.php";
+
+[$shortClass, $methodName] = explode('::', $subject);
+$method = new ReflectionMethod("GraphQL\\Benchmarks\\{$shortClass}", $methodName);
+$bench = $method->getDeclaringClass()->newInstance();
+
+mt_srand(1);
+$docComments = [$method->getDocComment(), $method->getDeclaringClass()->getDocComment()];
+foreach ($docComments as $docComment) {
+    if (is_string($docComment) && preg_match('/@BeforeMethods\((.+)\)/', $docComment, $beforeMethodsAnnotation) === 1) {
+        preg_match_all('/"(\w+)"/', $beforeMethodsAnnotation[1], $beforeMethods);
+        if ($beforeMethods[1] === []) {
+            throw new RuntimeException("Found no method names in @BeforeMethods: {$subject}.");
+        }
+
+        foreach ($beforeMethods[1] as $beforeMethodName) {
+            $beforeMethod = new ReflectionMethod($bench, $beforeMethodName);
+            $beforeMethod->invoke($bench);
+        }
+
+        break;
+    }
+}
+
+$benchmark = $method->getClosure($bench);
+for ($rev = 0; $rev < (int) $revs; ++$rev) {
+    $benchmark();
+}

phpbench.json

@@ -2,7 +2,6 @@
   "runner.bootstrap": "vendor/autoload.php",
   "runner.path": "benchmarks",
   "runner.file_pattern": "*Bench.php",
-  "runner.retry_threshold": 2,
   "runner.time_unit": "milliseconds",
   "runner.progress": "plain",
   "report.generators": {