refactor(benchmark): split the comparative harness from the CI one - #442
Merged
rmanibus merged 2 commits intoAug 4, 2026
Merged
Conversation
CI was running `run.sh local local cloudstic` — a 615-line cross-tool harness filtered down to one tool, on a runner where restic, borg and duplicacy are not installed. The comparison never happened there. The dead paths were the smaller problem. The real one is that the two jobs have conflicting requirements: a comparison needs a dataset that is fair across four tools and can only measure what all of them expose, while tracking this product over time needs a dataset that stresses this design and is free to report numbers no other tool has. Sharing one script meant a change made for cross-tool fairness would silently move the numbers CI trends on. - run.sh becomes compare.sh, which is only ever run by hand. - cloudstic.sh is new and is what the Benchmark workflow runs. It measures what a comparison cannot: incremental cost at one changed file and at a thousand, deduplication of already-stored data, and the stored-to-logical ratio. - lib.sh holds the measurement mechanics both use. The datasets deliberately stay separate — sharing them would reintroduce exactly the coupling this split removes. The report grows a Repo added column for a single-tool run, which is where deduplication shows up: copying 340 MB of already-stored data adds 40 KB. That column is withheld across tools, where it would invite comparing byte counts between different repository formats. Trigger is unchanged: manual dispatch. Worth noting the workflow has fired once since April, so the split is only worth what someone runs.
…memory The Benchmark workflow was dispatch-only, and had fired once since April. It now also runs on a pull request carrying the `benchmark` label, matching what Memory scaling already does. Deliberately the same label rather than a second one: labelling a PR is the act of asking for performance data, and wanting the memory curve but not the throughput numbers is not a real case. `synchronize` is included so pushing to an already-labelled PR re-measures without another click, and the job is skipped for any other label so an unrelated one does not start it.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stacked on #435 — base is
refactor/bench-reporting-in-go, so review that first. GitHub retargets this tomainautomatically once #435 merges. Per repo convention a stacked PR gets no CI run.What CI was actually doing
benchmark.ymlinvokedrun.sh local local cloudstic— the third argument filters to cloudstic — on a runner where restic, borg and duplicacy are not installed. The comparison never happened in CI. It was a 615-line cross-tool harness executing 64 lines of cloudstic-specific code.Why that matters beyond the dead paths
The dead paths are cosmetic. The structural problem is that the two jobs have conflicting requirements:
Sharing one script meant a change made for cross-tool fairness — adjusting the dataset so borg is not unfairly penalised, say — would silently move the numbers a trend line is built on.
The split
compare.sh(wasrun.sh)cloudstic.sh(new)lib.sh(new)lib.shholdsrun_bench, repo-size tracking and the table — the how. The datasets deliberately stay separate, which follows directly from the argument above: sharing them would reintroduce exactly the coupling the split removes.cloudstic.shmeasures things a comparison cannot, and the first run shows why they are worth having:Initial BackupIncremental (1 File Changed)Incremental (1000 Changed)Deduplicated BackupPruneCopying 340 MB of already-stored data adds 40 KB, and a thousand changed files costs the same wall time as one — both properties the design claims and neither previously measured. The script also reports a stored-to-logical ratio (2.18x on this dataset).
Report changes
The table grows a Repo added column for single-tool runs, and the prose no longer says "each tool over the same dataset" when there is one. The column is deliberately withheld across tools, where it would invite comparing byte counts between different repository formats — that is not the same measurement twice.
Verification
./scripts/benchmark/cloudstic.sh— full run, output above./scripts/benchmark/compare.sh local local cloudstic— still works after thelib.shextraction, output unchanged/bin/bash -non all three scripts under bash 3.2.57go test -race ./...passes;golangci-lint run ./...— 0 issuesnpx markdownlint-cli2 '**/*.md'— 0 issuesTrigger
The Benchmark workflow had fired once since April 2nd, so the split was worth roughly nothing on its own. It now also runs on a pull request carrying the
benchmarklabel, matching Memory scaling.Same label rather than a second one, deliberately: labelling a PR is the act of asking for performance data, and wanting the memory curve but not the throughput numbers is not a real case.
synchronizeis included so pushing to an already-labelled PR re-measures without another click, and any other label leaves both jobs skipped.