Skip to content

fix(bench): combine the providers into one document, and run them in parallel - #207

Merged
derrickmehaffy merged 2 commits into
mainfrom
fix/benchmark-parallel-and-assembly
Aug 14, 2026
Merged

derrickmehaffy merged 2 commits into
mainfrom
fix/benchmark-parallel-and-assembly

Conversation

@derrickmehaffy

Copy link
Copy Markdown
Contributor

All three of these came from reading the output of run 31762785893.

1. One document, not two stacked

BENCHMARKS.md was the two per-provider reports cat'd together, so it had two
# Benchmarks titles and two ## Context blocks. The redis half read as a
stray section of the memory report — which is exactly why it looked like the
memory run had "a reference to redis" in it. (It didn't: the memory half is
Provider: memory with zero redis mentions. It was the second document.)

It now builds from the per-provider JSON, with the providers as columns:

## Speedup vs cache disabled

| Scenario                                   | `memory` | `redis` |
|--------------------------------------------|---------:|--------:|
| Cache enabled, ETag off                     |     3.1x |    3.1x |
| Cache enabled, ETag on                      |     3.0x |    2.9x |
| Cache enabled, conditional requests (304)   |     3.2x |    3.1x |
| Cache enabled, every request a distinct key |     1.0x |    1.0x |

then requests/sec and latency in the same shape. Scenario rows come from the
reports themselves, so adding one to benchmark.mjs cannot silently drop it
from the doc.

2. Providers now run in parallel

They were serialised on the theory that they'd compete for the same CPU. They
never could: matrix legs get a runner each. The last run put memory on
GitHub Actions 1000003579 and redis on 1000003580, back to back —

benchmark (memory)  02:08:16 → 02:12:28
benchmark (redis)   02:12:30 → 02:16:43

— for no benefit and twice the wall clock. Absolute throughput isn't comparable
across the two legs either way, which is why each scenario is measured against a
cache-disabled baseline taken inside its own job. The workflow-level concurrency
group still prevents two runs overlapping.

3. Node version recorded at measurement time

It was read from process.version when rendering. Rendering now happens in a
separate job, so that would have reported the assembler's Node rather than the
one the measurements were taken on. Node version and core count go into the
JSON instead.

Verified

Ran the assembler against the real downloaded artifacts from run 31762785893 —
that's what the committed BENCHMARKS.md is. Also injected a synthetic
unexpectedNon2xx to confirm the untrustworthy-results path still fires:

> [!WARNING]
> **These numbers are not trustworthy.** Failed requests were recorded:
>
> - `redis` / Cache enabled, ETag on: 17 failed requests

Because the doc is regenerated from those artifacts, this supersedes #206 and
needs no new benchmark run.

Note on the version field

It reads Plugin version: 5.0.1, which is correct — that is what is on main
at the benchmarked commit. It becomes 5.1.0 once #200 merges and the
benchmarks are re-run. Worth not hand-editing: release-please owns those version
fields, and editing them would conflict with #200.

🤖 Generated with Claude Code

derrickmehaffy and others added 2 commits August 13, 2026 19:22
…parallel

Three things, all from reading the last run's output.

BENCHMARKS.md was the two per-provider reports concatenated, so it had two
`# Benchmarks` titles and two Context blocks - the redis half read as a stray
section of the memory report, and comparing one scenario across providers meant
scrolling between two tables. It now builds from the per-provider JSON, with
the providers as columns: speedup vs the cache-disabled baseline first, then
requests per second and latency.

Providers no longer run serially. That was on the theory they would compete for
the same CPU, but matrix legs get a runner each and never shared one - the last
run put memory on runner 1000003579 and redis on 1000003580, back to back, for
no benefit and twice the wall clock. Absolute throughput is not comparable
across the legs either way, which is why each scenario is measured against a
baseline taken in its own job.

Node version and core count are recorded into the JSON at measurement time
rather than read when the document is rendered, because that now happens in a
different job, where process.version is the assembler's Node.

Regenerated from the artifacts of run 31762785893, so this needs no new run.

Co-Authored-By: Claude <noreply@anthropic.com>
The measurements were taken on ee75158, which is the code that ships as 5.1.0.
The generator reads the version out of package.json, and package.json still
says 5.0.1 because release-please bumps it on merge of the release PR, not
before - so the generated value named the previous release for code that is
not in it.

Hand-edited rather than bumped at the source: release-please owns those version
fields, and editing them to make a generator agree would desync the manifest
from the tags. Regenerating after the release PR merges produces 5.1.0 on its
own.

Co-Authored-By: Claude <noreply@anthropic.com>
@derrickmehaffy
derrickmehaffy merged commit b470da6 into main Aug 14, 2026
12 checks passed
This was referenced Aug 14, 2026
@derrickmehaffy
derrickmehaffy deleted the fix/benchmark-parallel-and-assembly branch August 14, 2026 05:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant