From 1cce344f85ce9c073810b515a0f388ee896ac72d Mon Sep 17 00:00:00 2001 From: Dmitriy Kovalenko Date: Thu, 12 Nov 2026 12:53:36 -0800 Subject: [PATCH 1/2] ci(perf): compare benchmarks from each revision --- .github/workflows/performance-publish.yml | 2 +- .github/workflows/performance.yml | 6 +-- perf/README.md | 5 ++- perf/benchmark.sh | 18 +++++--- perf/build.sh | 9 ++-- perf/chart/examples/chart.rs | 39 ++++++++++++++--- perf/chart/src/lib.rs | 52 +++++++++++++---------- perf/report.sh | 32 ++++++++------ 8 files changed, 103 insertions(+), 60 deletions(-) diff --git a/.github/workflows/performance-publish.yml b/.github/workflows/performance-publish.yml index b75376df0..fb5f32ced 100644 --- a/.github/workflows/performance-publish.yml +++ b/.github/workflows/performance-publish.yml @@ -134,7 +134,7 @@ jobs: echo "$message" >"$report" else code=2 - if [[ "$RESULT" == success && -s "$timings/base.bench" && -s "$timings/head.bench" ]]; then + if [[ "$RESULT" == success && -f "$timings/base.bench" && -f "$timings/head.bench" ]]; then chart="pr-$PR_NUMBER.png" chart_url='' if [[ "$CHART" == success ]] && { gh release view perf-charts >/dev/null 2>&1 || diff --git a/.github/workflows/performance.yml b/.github/workflows/performance.yml index 8fe093221..d6c2aaf9d 100644 --- a/.github/workflows/performance.yml +++ b/.github/workflows/performance.yml @@ -110,16 +110,14 @@ jobs: - name: Fetch source and benchmark harness env: HARNESS: ${{ github.sha }} - BASE: ${{ needs.plan.outputs.base_sha }} REVISION: ${{ matrix.variant == 'base' && needs.plan.outputs.base_sha || needs.plan.outputs.head_sha }} run: | - [[ "$HARNESS" =~ ^[0-9a-f]{40}$ && "$BASE" =~ ^[0-9a-f]{40}$ && "$REVISION" =~ ^[0-9a-f]{40}$ ]] + [[ "$HARNESS" =~ ^[0-9a-f]{40}$ && "$REVISION" =~ ^[0-9a-f]{40}$ ]] repo=https://github.com/dmtrKovalenko/fff.git git init -q source - git -C source fetch -q --depth=1 "$repo" "$HARNESS" "$BASE" "$REVISION" + git -C source fetch -q --depth=1 "$repo" "$HARNESS" "$REVISION" git -C source show "$HARNESS:perf/build.sh" > "$RUNNER_TEMP/build.sh" git -C source -c core.hooksPath=/dev/null checkout -q --detach "$REVISION" - git -C source -c core.hooksPath=/dev/null checkout "$HARNESS" -- crates/fff-core/benches - name: Set up Zig uses: mlugg/setup-zig@v2 with: diff --git a/perf/README.md b/perf/README.md index f4dc5b198..c3b584ed2 100644 --- a/perf/README.md +++ b/perf/README.md @@ -19,12 +19,13 @@ present, new revisions are measured. External contributors' workflows require GitHub's approval before their jobs can start. The PR workflow compiles both revisions on GitHub-hosted runners using the -same benchmark sources and the release `zlob` feature. One job at a time runs +benchmark sources from each revision and the release `zlob` feature. One job at a time runs the resulting executables directly on `load` as the runner service user. When a timing moves by more than 10%, the worker takes two more measurements of that benchmark group and uses the median across the three runs. GitHub Actions updates the PR with the timings and fails the head status when a -benchmark slows down by more than 10%. After the PR workflow finishes, +shared benchmark slows down by more than 10%. Added and removed cases show their +available timing. Give a changed workload a new benchmark ID. After the PR workflow finishes, **Publish FFF performance** reads its comparison plan and timings, draws the chart, and posts the result. It runs from `main` for all PRs, including forks. diff --git a/perf/benchmark.sh b/perf/benchmark.sh index 2cd84234a..a74c0f611 100644 --- a/perf/benchmark.sh +++ b/perf/benchmark.sh @@ -78,6 +78,7 @@ stage() { mkdir "$dest" for bench in fuzzy_search grep parse definition glob bigram memmem; do binary="$source/${bench}_bench" + if [[ -f "$source/targets" ]] && ! grep -Fxq "${bench}_bench" "$source/targets"; then continue; fi [[ -f "$binary" && ! -L "$binary" && $(stat -c %s "$binary") -le 67108864 ]] install -m 0555 "$binary" "$dest/${bench}_bench" done @@ -86,6 +87,7 @@ stage() { run_bench() { local side=$1 bins=$2 lane=$3 bench=$4 filter=$5 fixture_path=$6 shift 6 + [[ -f "$bins/${bench}_bench" ]] || return 0 echo "Measuring $side $lane $bench" >&2 if ! ( cd "$work" || exit @@ -109,8 +111,11 @@ run_bench() { gsub(",", "", value) if (value <= 0 || ++count > 150) exit 1 print "test", lane "/" name, "... bench:", value, "ns/iter (+/-", $NF + next } - END { if (!count) exit 1 } + $1 == "test" { exit 1 } + NF { unexpected = 1 } + END { if (!count && unexpected) exit 1 } ' "$work/stdout" } @@ -135,7 +140,7 @@ sample_pair() { drifted() { awk ' - NR == FNR { base[$2] = $5; next } + FILENAME == ARGV[1] { base[$2] = $5; next } $2 in base && (10 * $5 > 11 * base[$2] || 10 * $5 < 9 * base[$2]) { found = 1 } END { exit !found } ' "$1" "$2" @@ -148,7 +153,7 @@ pair() { if [[ -n "$base" ]]; then sides+=(base) else - grep -F "test $1/$2/" "$output/base.bench" >"$work/base.1" + awk -v prefix="test $1/$2/" 'index($0, prefix) == 1' "$output/base.bench" >"$work/base.1" fi if drifted "$work/base.1" "$work/head.1"; then echo "Rechecking $1 $2 after a change over 10%" >&2 @@ -222,8 +227,11 @@ pair core glob '^glob_(reuse_10k|mixed_100k)/' "$path" pair core bigram '^bigram_query_100000/(medium_6char|long_14char|multi_word)$|^bigram_build/short_content/10000$' "$path" pair core memmem '/file_picker\.rs/53KB/(medium|long)/miss$' "$path" -diff -u <(awk '{print $2}' "$output/base.bench" | sort) \ - <(awk '{print $2}' "$output/head.bench" | sort) +[[ -s "$output/base.bench" || -s "$output/head.bench" ]] +if [[ "$mode" != compare ]]; then + diff -u <(awk '{print $2}' "$output/base.bench" | sort) \ + <(awk '{print $2}' "$output/head.bench" | sort) +fi if [[ "$mode" == calibrate ]]; then cp "$output/base.bench" "$work/reference/baseline.bench" sha256sum "$0" | cut -d' ' -f1 >"$work/reference/script.sha256" diff --git a/perf/build.sh b/perf/build.sh index 71eaa82ec..0010d93d9 100644 --- a/perf/build.sh +++ b/perf/build.sh @@ -6,10 +6,6 @@ mkdir -p "$2" output=$(realpath "$2") target=x86_64-unknown-linux-musl benches=(fuzzy_search_bench grep_bench parse_bench definition_bench glob_bench bigram_bench memmem_bench) -bench_args=() -for name in "${benches[@]}"; do - bench_args+=(--bench "$name") -done mkdir "$output/work" "$output/bins" docker build --tag fff-perf-build - <<'DOCKERFILE' @@ -39,7 +35,7 @@ docker run --rm --user "$(id -u):$(id -g)" --cap-drop ALL \ --env PATH=/zig:/rust/bin:/tools:/usr/local/bin:/usr/bin:/bin \ --workdir /src fff-perf-build \ cargo zigbuild --locked --profile ci --target "$target" -p fff-search \ - --no-default-features --features zlob "${bench_args[@]}" --message-format=json-render-diagnostics \ + --no-default-features --features zlob --benches --message-format=json-render-diagnostics \ | jq -Rr 'fromjson? | if .reason == "compiler-message" then .message.rendered // empty elif .reason == "compiler-artifact" and .target.kind == ["bench"] and .executable then [.target.name, .executable] | @tsv else empty end' > "$output/build.log" || { @@ -47,12 +43,15 @@ docker run --rm --user "$(id -u):$(id -g)" --cap-drop ALL \ exit 1 } +: > "$output/bins/targets" for name in "${benches[@]}"; do binary=$(awk -F '\t' -v name="$name" '$1 == name { print $2 }' "$output/build.log") + [[ -n "$binary" ]] || continue [[ "$binary" =~ ^/build/target/$target/ci/(deps/)?${name}(-[0-9a-f]+)?$ ]] binary=$(realpath -e "$output/work/${binary#/build/}") [[ "$binary" == "$output/work/target/$target/ci/"* && -f "$binary" ]] headers=$(readelf -l "$binary") [[ "$headers" != *INTERP* ]] install -m 0555 "$binary" "$output/bins/$name" + echo "$name" >> "$output/bins/targets" done diff --git a/perf/chart/examples/chart.rs b/perf/chart/examples/chart.rs index 8e1012ac7..55161b750 100644 --- a/perf/chart/examples/chart.rs +++ b/perf/chart/examples/chart.rs @@ -22,6 +22,7 @@ const DIM: &str = "#74747e"; const INK: &str = "#18181b"; const GREEN: &str = "#17a34a"; const RED: &str = "#e5484d"; +const BLUE: &str = "#2563b0"; const MARGIN: f32 = 60.0; const LABEL_X: f32 = 84.0; @@ -46,13 +47,17 @@ struct Chart { impl Chart { fn new(timings: Vec) -> Self { - let worst = timings.iter().map(|t| t.change().abs()).fold(0.0, f64::max); + let worst = timings + .iter() + .filter_map(Timing::change) + .map(f64::abs) + .fold(0.0, f64::max); let span = (worst / 10.0).ceil().clamp(2.0, 10.0) as f32 * 10.0; let mut groups: Vec = Vec::new(); for timing in timings { let name = group_name(&timing.name); - match groups.last_mut() { - Some(group) if group.name == name => group.timings.push(timing), + match groups.iter_mut().find(|group| group.name == name) { + Some(group) => group.timings.push(timing), _ => groups.push(Group { name, timings: vec![timing], @@ -84,7 +89,9 @@ impl Chart { let faster = all() .filter(|t| matches!(t.verdict(), Verdict::Faster)) .count(); - let within = all().count() - slower - faster; + let within = all() + .filter(|t| matches!(t.verdict(), Verdict::Within)) + .count(); let mut x = MARGIN; let legend: Vec<_> = [ (RED, format!("{slower} slower")), @@ -174,12 +181,14 @@ impl Chart { } fn row(&self, timing: &Timing, y: f32) -> Svgr<'static> { - let change = timing.change() as f32; let (fill, text, weight) = match timing.verdict() { Verdict::Slower => (RED, INK, 600), Verdict::Faster => (GREEN, INK, 600), Verdict::Within => (EDGE, DIM, 400), + Verdict::New(ns) => return self.unpaired_row(timing, y, BLUE, "NEW", ns), + Verdict::Removed(ns) => return self.unpaired_row(timing, y, DIM, "REMOVED", ns), }; + let change = timing.change().unwrap() as f32; let (from, to) = (self.x(change.min(0.0)), self.x(change.max(0.0))); svgr!( @@ -197,6 +206,24 @@ impl Chart { ) } + + fn unpaired_row( + &self, + timing: &Timing, + y: f32, + color: &'static str, + status: &'static str, + ns: f64, + ) -> Svgr<'static> { + let elapsed = std::time::Duration::from_secs_f64(ns / 1e9); + svgr!( + + {label(&timing.name)} + {status.to_owned()} + {format!("{elapsed:.2?}")} + + ) + } } impl Group { @@ -265,7 +292,7 @@ fn render(base: &str, head: &str, output: &str) -> Result<(), Box, + head: Option, } impl Timing { /// PR time against base in percent, `+12.0` is 12% slower. - pub fn change(&self) -> f64 { - 100.0 * (self.head / self.base - 1.0) + pub fn change(&self) -> Option { + let base = self.base?; + Some(100.0 * (self.head? - base) / base) } /// Same 10% limits as `perf/report.sh`. pub fn verdict(&self) -> Verdict { - if 10.0 * self.head > 11.0 * self.base { - Verdict::Slower - } else if 10.0 * self.head < 9.0 * self.base { - Verdict::Faster - } else { - Verdict::Within + match (self.base, self.head) { + (None, Some(ns)) => Verdict::New(ns), + (Some(ns), None) => Verdict::Removed(ns), + (Some(b), Some(h)) if 10.0 * h > 11.0 * b => Verdict::Slower, + (Some(b), Some(h)) if 10.0 * h < 9.0 * b => Verdict::Faster, + _ => Verdict::Within, } } } -/// Pairs the bencher lines of both files by name, in the order of `head`. pub fn parse(base: &str, head: &str) -> Vec { - let base: Vec<_> = base.lines().filter_map(line).collect(); - head.lines() + let mut base: std::collections::BTreeMap<_, _> = base.lines().filter_map(line).collect(); + let mut timings: Vec<_> = head + .lines() .filter_map(line) - .filter_map(|(name, head)| { - let (_, base) = base.iter().find(|(known, _)| *known == name)?; - Some(Timing { - name: name.to_owned(), - base: *base, - head, - }) + .map(|(name, head)| Timing { + name: name.to_owned(), + base: base.remove(name), + head: Some(head), }) - .collect() + .collect(); + timings.extend(base.into_iter().map(|(name, base)| Timing { + name: name.to_owned(), + base: Some(base), + head: None, + })); + timings } // "test ... bench: ns/iter (+/- )" fn line(line: &str) -> Option<(&str, f64)> { let mut fields = line.split_whitespace(); let name = fields.nth(1)?; - let value: f64 = fields.nth(2)?.parse().ok()?; - (value > 0.0).then_some((name, value)) + let value: u64 = fields.nth(2)?.parse().ok()?; + (value > 0).then_some((name, value as f64)) } diff --git a/perf/report.sh b/perf/report.sh index 3a136db91..62ee9b7cd 100644 --- a/perf/report.sh +++ b/perf/report.sh @@ -5,6 +5,7 @@ export LC_ALL=C # Prints the markdown report, exits 1 when a benchmark slowed down by over 10%. awk -v chart="${3:-}" ' function human(ns) { + if (!ns) return "—" if (ns >= 1e9) return sprintf("%.2f s", ns / 1e9) if (ns >= 1e6) return sprintf("%.2f ms", ns / 1e6) if (ns >= 1e3) return sprintf("%.2f µs", ns / 1e3) @@ -16,25 +17,28 @@ awk -v chart="${3:-}" ' fence = "`" while (index(bench, fence)) fence = fence "`" if (length(fence) > 1) bench = " " bench " " - return sprintf("| %s | %s%s%s | %s | %s | %+.2f%% |\n", mark[i], fence, bench, fence, human(before[i]), human(after[i]), ratio[i] - 100) + return sprintf("| %s | %s%s%s | %s | %s | %s |\n", mark[i], fence, bench, fence, human(base[name[i]]), human(head[name[i]]), change[i]) } $1 != "test" || $3 != "..." || $4 != "bench:" || $5 !~ /^[0-9]+$/ || $5 <= 0 || $6 != "ns/iter" { next } - NR == FNR { base[$2] = $5; next } - $2 in base { - n++ - name[n] = $2 - before[n] = base[$2] - after[n] = $5 - ratio[n] = 100 * $5 / base[$2] - mark[n] = "⚪" - if (10 * $5 > 11 * base[$2]) { mark[n] = "🔴"; slower++ } - if (10 * $5 < 9 * base[$2]) { mark[n] = "🟢"; faster++ } - } + !seen[$2]++ { name[++n] = $2 } + FILENAME == ARGV[1] { base[$2] = $5; next } + { head[$2] = $5 } END { if (!n) exit 2 + for (i = 1; i <= n; i++) { + b = base[name[i]]; h = head[name[i]] + if (!b) { mark[i] = "🔵"; change[i] = "New"; added++; continue } + if (!h) { mark[i] = "➖"; change[i] = "Removed"; removed++; continue } + change[i] = sprintf("%+.2f%%", 100 * (h - b) / b) + if (10*h > 11*b) { mark[i] = "🔴"; slower++ } + else if (10*h < 9*b) { mark[i] = "🟢"; faster++ } + else { mark[i] = "⚪"; within++ } + } header = "| | Benchmark | Base | PR | Change |\n| --- | --- | ---: | ---: | ---: |\n" - printf "🔴 %d slower, 🟢 %d faster, ⚪ %d within ±10%%\n\n", slower, faster, n - slower - faster - if (slower + faster) { + printf "🔴 %d slower, 🟢 %d faster, ⚪ %d within ±10%%", slower, faster, within + if (added || removed) printf ", %d new, %d removed", added, removed + print "\n" + if (slower + faster + added + removed) { printf "%s", header for (i = 1; i <= n; i++) if (mark[i] != "⚪") printf "%s", row(i) print "" From e02308af2928a11bf1cfa76715d7465d49b6ff08 Mon Sep 17 00:00:00 2001 From: Dmitriy Kovalenko Date: Thu, 12 Nov 2026 10:27:03 -0800 Subject: [PATCH 2/2] fix(parser): simplify the wildcard check for Clippy 1.100 --- crates/fff-query-parser/src/config.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/crates/fff-query-parser/src/config.rs b/crates/fff-query-parser/src/config.rs index 9516c96db..e2d9ae468 100644 --- a/crates/fff-query-parser/src/config.rs +++ b/crates/fff-query-parser/src/config.rs @@ -203,15 +203,10 @@ impl ParserConfig for AiGrepConfig { return false; } let bytes = token.as_bytes(); - if bytes.len() >= 3 + bytes.len() >= 3 && bytes[0] == b'*' && bytes[bytes.len() - 1] == b'*' - && bytes[1..bytes.len() - 1].iter().all(|&b| b != b'*') - { - return true; - } - - false + && !bytes[1..bytes.len() - 1].contains(&b'*') } }