diff --git a/.github/workflows/benchmarks.yml b/.github/workflows/benchmarks.yml index 7f6ca32de..9cc4949d5 100644 --- a/.github/workflows/benchmarks.yml +++ b/.github/workflows/benchmarks.yml @@ -2,6 +2,11 @@ name: Benchmarks on: pull_request: + paths: + - .github/workflows/benchmarks.yml + - benchmarks/** + - composer.json + - src/** push: branches: - master @@ -9,15 +14,26 @@ on: 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: @@ -25,30 +41,25 @@ jobs: 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 diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ca39583e5..bdfb8a403 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -86,6 +86,7 @@ jobs: benchmarks: name: Benchmarks runs-on: ubuntu-latest + timeout-minutes: 20 steps: - name: Checkout diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c941652c4..528e44391 100644 --- a/CONTRIBUTING.md +++ b/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). diff --git a/benchmarks/compare-results.php b/benchmarks/compare-results.php new file mode 100644 index 000000000..5409b69fe --- /dev/null +++ b/benchmarks/compare-results.php @@ -0,0 +1,48 @@ + \n"); + exit(1); +} + +$threshold = 1.1; + +/** @var array $base */ +$base = json_decode((string) file_get_contents($baseFile), true, 2, JSON_THROW_ON_ERROR); +/** @var array $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); diff --git a/benchmarks/count-instructions.php b/benchmarks/count-instructions.php new file mode 100644 index 000000000..29210cb6c --- /dev/null +++ b/benchmarks/count-instructions.php @@ -0,0 +1,100 @@ +\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 $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"; diff --git a/benchmarks/run-subject.php b/benchmarks/run-subject.php new file mode 100644 index 000000000..fd0119f64 --- /dev/null +++ b/benchmarks/run-subject.php @@ -0,0 +1,39 @@ + \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(); +} diff --git a/phpbench.json b/phpbench.json index de5b5c6f5..28c39ada6 100644 --- a/phpbench.json +++ b/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": {