Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,17 @@ jobs:
if [ "$found" = 0 ]; then
echo "no results directories yet, nothing to check"
fi
# The M8 exit gate, run rather than asserted.
# The committed output.json is beside the charts so that anybody can redraw them without trusting us, and that promise is only worth something if somebody redraws them. This redraws every chart in memory from the committed data and compares SHA-256 against the committed manifest, which is a text file diff rather than 146 pictures.
# It is also the check that catches a change to the chart code that quietly moves a pixel, which is the failure nothing else here can see.
# This runner is Linux and the directories were drawn on Linux and on macOS, so a mismatch here is either the chart code moving or the two platforms disagreeing, and both are worth stopping for.
- name: every chart redraws from the committed data
run: |
shopt -s nullglob
for dir in results/*/; do
echo "redrawing $dir"
cargo run --locked --release --package cache-bench -- chart --dir "$dir" --check "$dir/graphs.sha256"
done

# The floor stays green too.
# It is the same floor as tamnd/yo, so the two trees agree on what a supported compiler is.
Expand Down
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@

What each release costs you, in the order the releases happened. New entries go on top.

## 0.6.1 - unreleased

### Fixed

- `chart --dir` drew unstamped charts when `--profile` was left off, and the stamp is part of the picture, so every chart came out different from the committed one. Checking a published directory that way reported that all 146 of its charts failed the manifest, which reads as the charts not being reproducible when what happened is that an argument was missing. The profile now comes off the directory's own `host.json` when there is no `--profile`, which is the only right answer for a results directory and one nobody can get wrong. A test says every published directory is named for the profile it records, since the two disagreeing would put one name on the directory and another on the charts inside it.
- The spell check failed on a Redis build id. Every engine's version line is recorded verbatim, because a chart is only worth anything if it says what it measured, and Redis 8.10.1 on the 8 core host prints a build id with a two letter run in the middle of it that reads as a misspelling of an English word. Nothing here writes those strings and none of them can be corrected, so the hash is skipped rather than the file it sits in, which keeps the prose around it checked. The first host got through on luck rather than on anything the check knew.
- Nothing ran the check that redrawing the charts from the committed `output.json` reproduces the committed PNGs, which is the whole reason `output.json` is committed beside them and is M8's exit gate written out as a sentence. CI now runs it for every published directory, in the job that already checks the generated documents, comparing SHA-256 rather than pictures. It is also the only check here that can see a change to the chart code quietly moving a pixel.

## 0.6.0 - 2026-09-10

### Added
Expand Down
50 changes: 45 additions & 5 deletions crates/cache-bench/src/chart.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use std::path::{Path, PathBuf};

use cb_chart::render::{Stamp, draw};
use cb_chart::{Axis, Chart, Corpus, Scale, Spec};
use cb_core::{Compat, Output, Profiles};
use cb_core::{Compat, Machine, Output, Profiles};

/// All 154 charts as the original drew them, which is what `--golden` draws from.
use cb_core::golden::SERIES;
Expand All @@ -33,7 +33,7 @@ pub(crate) struct Args {
/// Check what was drawn against a manifest written earlier, and draw nothing to disk.
#[arg(long, value_name = "PATH", conflicts_with_all = ["out", "manifest"])]
check: Option<PathBuf>,
/// Which hardware profile produced the numbers, for the stamp along the bottom.
/// Which hardware profile produced the numbers, for the stamp along the bottom. Read from the results directory's `host.json` when there is one.
#[arg(long, value_name = "NAME")]
profile: Option<String>,
/// Where to read the profiles from.
Expand Down Expand Up @@ -155,9 +155,16 @@ fn destination(args: &Args) -> Result<Option<PathBuf>, String> {

/// The stamp that goes along the bottom of every chart.
///
/// Nothing without `--profile`, which is what keeps a chart drawn from the golden series byte identical everywhere. A chart drawn from real measurements should always carry one, and `doctor` is where the sweep is told to.
/// Nothing without a profile, which is what keeps a chart drawn from the golden series byte identical everywhere. A chart drawn from real measurements should always carry one, and `doctor` is where the sweep is told to.
///
/// A results directory names its own profile in `host.json`, so `--profile` is only needed when there is no directory to read it from. Passing the wrong one is impossible in the ordinary case and forgetting it is no longer silent: it used to draw 146 unstamped charts and report that all 146 of them failed the manifest, which says the charts are not reproducible when what happened is that the reader left an argument off.
fn stamp(args: &Args) -> Result<Stamp, String> {
let Some(name) = &args.profile else {
let named = match (&args.profile, &args.dir) {
(Some(name), _) => Some(name.clone()),
(None, Some(dir)) => Some(profile_of(dir)?),
(None, None) => None,
};
let Some(name) = &named else {
return Ok(Stamp::default());
};
let text = fs::read_to_string(&args.profiles)
Expand All @@ -175,6 +182,19 @@ fn stamp(args: &Args) -> Result<Stamp, String> {
})
}

/// The profile a results directory says it was measured under.
fn profile_of(dir: &Path) -> Result<String, String> {
let path = dir.join("host.json");
let text = fs::read_to_string(&path).map_err(|e| {
format!(
"{}: {e}, so pass --profile to say which one drew these",
path.display()
)
})?;
let machine = Machine::parse(&text).map_err(|e| format!("{}: {e}", path.display()))?;
Ok(machine.profile)
}

/// Which scale a chart is drawn on, which its filename says.
fn scale_of(file: &str) -> Scale {
if file.contains("scale_logarithmic") {
Expand Down Expand Up @@ -265,10 +285,30 @@ pub(crate) fn expected() -> usize {
#[cfg(test)]
#[allow(clippy::expect_used, reason = "a failed fixture is a failed test")]
mod tests {
use super::{digest, expected, parse_manifest, render_manifest, scale_of};
use super::{digest, expected, parse_manifest, profile_of, render_manifest, scale_of};
use cb_chart::Scale;
use std::collections::BTreeMap;

// Every published directory is named for the profile it was measured under, and the stamp along the bottom of its charts now comes off the file inside rather than off the name. If those two ever disagree, a directory called one thing carries charts stamped another, and the manifest beside it stops reproducing for anybody who passes the name they can see.
#[test]
fn a_results_directory_is_named_for_the_profile_it_says_it_ran() {
let root = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../../results");
let Ok(entries) = std::fs::read_dir(&root) else {
return;
};
for entry in entries.flatten() {
let dir = entry.path();
if !dir.join("host.json").exists() {
continue;
}
let named = dir
.file_name()
.and_then(|name| name.to_str())
.expect("a directory has a name");
assert_eq!(profile_of(&dir).expect("host.json reads"), named);
}
}

#[test]
fn the_scale_comes_off_the_filename() {
assert_eq!(
Expand Down
4 changes: 4 additions & 0 deletions typos.toml
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
[default]
# Every engine's version line is recorded verbatim, because a chart is only worth something if it says what it measured, and those lines carry git hashes and build ids. Hex sometimes spells a word: Redis 8.10.1 on the 8 core host prints a build id with a two letter run in the middle of it that this checker reads as a misspelling of an English word. Nothing in this repository writes those strings and none of them can be corrected, so the hash itself is skipped rather than the file it sits in, which keeps the prose around it checked.
extend-ignore-re = ["(sha|build)=[0-9a-f]{6,}"]

[default.extend-words]
# The original spells memcached this way in its filenames and we match it.
memcache = "memcache"
Expand Down