Skip to content

Commit 3ec9960

Browse files
committed
fix(docs): retry snapshot push; portable Pages URL; drop non-positive Y
Three code-review fixes on PR 5: 1. Snapshot push race. The bench-upload Snapshot step pushed devel without rebasing, so a concurrent push during a long bench run would reject non-fast-forward and fail the entire job. Add a fetch + rebase + retry loop (3 attempts) and push HEAD:devel. The commit is already [skip ci], so the retry cannot trigger an infinite bench loop on a successful push. 2. Hardcoded GitHub Pages verify URL. The docs.yml verification step curled https://elijahr.github.io/lockfreequeues/... which is wrong on forks or after a repo rename. Derive the URL from github.repository_owner and github.event.repository.name so the verifier follows the repo wherever it lives. 3. Log-scale accepts non-positive. The chart defaults to log Y (distr: 3) but merge_bmf.py only enforces finiteness, so a malformed BMF could deliver a 0 or negative throughput value. Treat <= 0 as missing data in toUplotData. Throughput in ops/ms cannot legitimately be non-positive (a 0 indicates a degenerate run that bench_throughput's elapsedNs guard already rejects), so surfacing as missing is the correct semantics.
1 parent 4adc5bd commit 3ec9960

3 files changed

Lines changed: 44 additions & 5 deletions

File tree

.github/workflows/bench.yml

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -709,4 +709,25 @@ jobs:
709709
exit 0
710710
fi
711711
git commit -m "chore(bench): refresh snapshot [skip ci]"
712-
git push origin devel
712+
# Long bench runs leave a wide window for `devel` to advance
713+
# before we push (a docs-only follow-up commit, an unrelated
714+
# merge, etc.); a plain `git push` would then be rejected
715+
# non-fast-forward and fail the entire bench-upload job.
716+
# Retry: fetch+rebase+push up to 3 times. Skip-CI is on the
717+
# commit so the retry loop cannot trigger an infinite bench
718+
# workflow regardless of how many fast-forwards happen.
719+
for attempt in 1 2 3; do
720+
if git push origin HEAD:devel; then
721+
exit 0
722+
fi
723+
echo "::notice::push attempt $attempt rejected; rebasing onto origin/devel and retrying."
724+
git fetch origin devel
725+
# Rebase preserves our snapshot commit on top of any new
726+
# commits that landed since checkout. Conflict on the
727+
# snapshot files themselves is impossible (no other workflow
728+
# writes to docs/assets/bench-results/), so a clean rebase
729+
# is the expected outcome.
730+
git rebase origin/devel
731+
done
732+
echo "::error::Snapshot push failed after 3 retries; aborting." >&2
733+
exit 1

.github/workflows/docs.yml

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,13 +67,23 @@ jobs:
6767
# `sleep 30` accommodates GitHub Pages CDN propagation; raise to
6868
# 60 if this step ever flakes.
6969
- name: Verify mike asset path
70+
# Derive the Pages URL from the repository context so a fork or
71+
# repo rename does not falsely fail this verification step. The
72+
# default GitHub Pages URL convention is
73+
# `https://<owner>.github.io/<repo>/`. If the project ever moves
74+
# to a custom domain the URL becomes wrong again, but at that
75+
# point we'd update the workflow to read site_url from
76+
# mkdocs.yml; for now the convention is stable.
7077
if: |
7178
github.ref == 'refs/heads/main' ||
7279
github.ref == 'refs/heads/master' ||
7380
github.ref == 'refs/heads/devel'
81+
env:
82+
OWNER: ${{ github.repository_owner }}
83+
REPO: ${{ github.event.repository.name }}
7484
run: |
7585
set -euo pipefail
76-
URL="https://elijahr.github.io/lockfreequeues/dev/assets/bench-results/latest.json"
86+
URL="https://${OWNER}.github.io/${REPO}/dev/assets/bench-results/latest.json"
7787
sleep 30
7888
HTTP_CODE=$(curl -sS -o /tmp/latest.json -w "%{http_code}" "$URL")
7989
if [ "$HTTP_CODE" != "200" ]; then

docs/assets/bench-charts.js

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,16 +134,24 @@
134134

135135
/* Convert {xLabels, libraries} into the column-major arrays uPlot
136136
* expects: data[0] is the X axis; data[i+1] is the Y series for
137-
* libraries[i]. Missing (library, shape) cells are NaN, which uPlot
138-
* skips in line drawing.
137+
* libraries[i]. Missing (library, shape) cells are null, which uPlot
138+
* skips in line drawing. Non-positive values are also treated as
139+
* null: throughput in ops/ms cannot legitimately be <= 0 (a 0 means
140+
* a degenerate run our throughput.nim guards already reject), and
141+
* the chart's default Y axis is log-scale (`distr: 3`) which is
142+
* undefined for non-positive values. merge_bmf.py only enforces
143+
* finiteness, so a malformed BMF could in principle deliver a 0 or
144+
* negative value; rather than letting it break the log axis,
145+
* surface it as missing data.
139146
*/
140147
function toUplotData(xLabels, libraries) {
141148
const x = xLabels.map((_, i) => i + 1);
142149
const series = libraries.map((lib) => {
143150
const byShape = new Map(lib.points.map((p) => [p.shape, p]));
144151
return xLabels.map((label) => {
145152
const p = byShape.get(label);
146-
return p ? p.value : null;
153+
if (!p) return null;
154+
return p.value > 0 ? p.value : null;
147155
});
148156
});
149157
return [x, ...series];

0 commit comments

Comments
 (0)