test: --save pipeline check for the per-framework results layout - #1035
Closed
MDA2AV wants to merge 4 commits into
Closed
test: --save pipeline check for the per-framework results layout#1035MDA2AV wants to merge 4 commits into
MDA2AV wants to merge 4 commits into
Conversation
Closes #751. Results lived in 52 shared arrays — site/data/<profile>-<conns>.json, each holding every framework — so a saved run rewrote files that every other framework also occupies. They now live one file per framework: site/data/baseline-512.json [ {actix…}, {hyper…}, … ] -> site/data/results/actix.json { framework, results: { "baseline-512": {…} } } 108 files, 1941 rows, same rows verbatim. The conflict is real but narrower than "any two saves": git merges non-adjacent hunks fine, so two frameworks updating existing rows usually merge. It bites when rows are *inserted* — two contributors each adding a framework, which is exactly when a rebase is most annoying. Reproduced both ways against the real data: old layout, actix + hyper update rows merges cleanly old layout, two new frameworks added CONFLICT in baseline-512 and -4096 new layout, the same two new frameworks merges cleanly, both files present Rows are copied verbatim rather than reshaped, which makes the migration provable: regenerating data.js before and after gives a byte-identical file. That is the main guarantee here — the board, the composite and every archived round see exactly what they saw before. Consumers: rebuild_site_data.py writes each framework's file; a run touches only the frameworks it benchmarked (verified: benchmarking actix + hyper modified 2 of 108 files) gen_leaderboard_data loads site/data/results/*.json once and rebuilds the per-profile view, sorted by framework name so the emitted data.js does not churn compare.sh one read of the framework's own file instead of a scan of every profile file archive.sh regroups into the existing round shape, so rounds/*.json keeps its current format benchmark-pr.yml stages only site/data/results/<slug>.json The filename is the display_name lowercased with anything outside [A-Za-z0-9._-] replaced by '-', because two gateway entries are named "aspnet-minimal + nginx" and "+ caddy". .gitignore's `results/` rule was unanchored and silently swallowed the new directory — the first attempt committed the code and none of the data. It is now `/results/`, which is the raw benchmark output it was always meant to cover; the root directory is still ignored. scripts/migrate_results_layout.py is kept rather than deleted: it re-derives the layout from a flat checkout, which is what a branch predating this needs in order to rebase. Note for #1020, which edits rebuild_site_data.py and deletes rows from the flat files: it will need a rebase, and running the migration script after merging main is the mechanical way to do it.
The step already declared env: GH_TOKEN at the end, so adding a second env: block for FRAMEWORK produced two env keys in one step. PyYAML accepts that silently (last one wins), which is why the local check passed, but GitHub rejects the whole file: 'env is already defined'. Merged into the existing block, and validated all six workflows with a loader that errors on duplicate keys rather than quietly dropping one.
Owner
Author
|
Closing — this was branched off |
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
1 participant
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.
Pipeline test for #1034 — please don't merge until we've looked at the result.
Branched from
per-framework-results, so its tree already has the per-framework results layout. The framework change itself is small and real (apt list cleanup +--no-install-recommendsin bjoern's image layer), so the run has something legitimate to build.What we're checking
A
--saverun should update onlysite/data/results/bjoern.json— specifically itsbaseline-512andbaseline-4096keys (currentlybaseline-512= 12,390 rps) — and touch no other framework's file. Under the old layout this would have rewrittensite/data/baseline-512.jsonandbaseline-4096.json, which every framework shares.How it has to be triggered
/benchmarkcomments runpr-commands.ymlfrom the default branch, which then dispatchesbenchmark-pr.ymlalso from the default branch. So commenting here would runmain's commit step (staging the old flat paths) against this branch's scripts (writing per-framework files) — it would save nothing and prove nothing.To exercise #1034's own workflow code, dispatch it explicitly against that branch:
It still requires your approval on the
runnerenvironment before anything executes.Once #1034 is merged this stops being necessary —
benchmark-pr.ymlalready mergesmaininto the PR branch on--saveruns, so ordinary/benchmarkcomments pick the new layout up automatically.🤖 Generated with Claude Code