doc: Demonstrate running benchmarks on each commit #35390

Comparing the before/after impact of a patch on the results of a benchmark or subset of benchmarks is useful information, especially to catch performance regressions or measure improvements before making a pull request. This is an adaption of the doc comment in productivity.md that explicitly shows how to run benches for each commit since diverging from master.

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--006a51241073e994b41acfe9ec718e94-->

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35390.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

<!--5faf32d7da4f0f540f40219e4f7537a3-->

This seems like a useful tool, but I'm worried it'll go stale soon if it's not part of some Bitcoin Core (e.g. CI, release process, ...) So I think it should either be integrated into the project (and this PR evaluated on those merits), or not be in the contrib/ directory to avoid that becoming a repository of stale code? This might warrant an RFC issue imho.

Not sure about adding a rust tool to our contrib scripts. This might as well be a dependency-less python script. Does typing really matter here?

Seems fine to add, but I think the only thing this does is transpose the result table and do some pretty printing of the delta.

Also, it only works to compare two commits and wouldn't work in a pull request with more than one commit.

Personally, I'd just prefer a bash one-liner, similar to:

> /tmp/bench.txt && git rebase --interactive HEAD~4 --exec 'cmake --build ./bld -j$(nproc) && git log -1 >> /tmp/bench.txt && ./bld-cmake/bin/bench_bitcoin --filter=.*Hex.* >> /tmp/bench.txt'
cat /tmp/bench.txt

This also has the benefit of not having to hard-code the cmake args.

Okay, wrote something locally that serves my use case of catching obvious regressions with a shell script. It prints the benchmark result for each commit from HEAD down to a specified base (e.g. master).

The downside of course is that this is not OS-agnostic, but I'm not sure if that matters.

#!/usr/bin/env bash
# Walk every commit from <BASE>..HEAD, build bench_bitcoin, and print results.
# Usage: compare-benches.sh [FILTER] [BASE] [BUILD_DIR]

set -euo pipefail

FILTER="${1:-.*}"
BASE="${2:-master}"
BUILD_DIR="${3:-build}"

BRANCH=$(git symbolic-ref --short HEAD 2>/dev/null || git rev-parse HEAD)
MERGE_BASE=$(git merge-base "$BASE" HEAD)

cleanup() {
    git checkout --quiet "$BRANCH"
}
trap cleanup EXIT

for sha in $(git log --reverse --format="%H" "${MERGE_BASE}..HEAD"); do
    git checkout --quiet --detach "$sha"

    COMMIT=$(git log -1 --pretty="%h  %s")
    printf '\n==> %s\n' "$COMMIT"
    printf '    cmake: building bench_bitcoin...\n'
    cmake --build "$BUILD_DIR" -j"$(nproc)" --target bench_bitcoin &>/dev/null

    "$BUILD_DIR/bin/bench_bitcoin" -filter=".*${FILTER}.*"
done

Example use:

contrib/compare-benches/compare-benches.sh "(MemPool|Orphanage)"

Inspired by the comment of @maflcko above. Are Linux-only scripts useful for contrib? This runs a lower risk of becoming stale but accomplishes what I'm going for.

Hmm, the Rust code is also hard-coding the bench name without exe, so it won't work on Windows either.

About the Bash script: I don't think we have any human Bash-script reviewer currently, so adding it is currently not possible. Also, it seems redundant with the already documented workflow in:

doc/productivity.md-To execute `cmake --build build && ctest --test-dir build` on every commit since last diverged from master, but without rebasing over an updated master, we can do the following:
doc/productivity.md-```sh
doc/productivity.md:git rebase -i --exec "cmake --build build && ctest --test-dir build" "$(git merge-base master HEAD)"
doc/productivity.md-```

Basically the only difference is that the bench tests are called instead of the unit tests. But the docs give just one example, which is extensible to call any tool, test, or utility (such as signing commits, etc).

Again, it seems fine to add, but it may be more flexible to just rely on the existing docs.

I would prefer having something explicit like:

git rebase -i --exec "cmake --build build && build/bin/bench_bitcoin -filter='.*MemPool.*'" "$(git merge-base master HEAD)"

mentioned in the benchmarking.md doc to make this more obvious.

I can update this PR with the doc addition and dump the contrib changes as this also does what I intend.

This seems like a useful tool, but I'm worried it'll go stale soon if it's not part of some Bitcoin Core (e.g. CI, release process, ...)

Tracking regressions or improvements on a regular cadence for each bench seems generally useful to me. Perhaps a start would be to add more pages to the benchcoin website or make a new one with a cheap-ish VPS.

This might warrant an RFC issue imho

Request for comment on scripts/automation related to benchmarking? Or what did you have in mind?

Tracking regressions or improvements on a regular cadence for each bench seems generally useful to me. Perhaps a start would be to add more pages to the benchcoin website or make a new one with a cheap-ish VPS.

Have you seen the section about corecheck in #35390 (comment) ?

Neat, way ahead of me.

Updated to:

    BASE=$(git merge-base master HEAD) && \
    git checkout --detach --quiet "$BASE" && \
    cmake --build build && \
    build/bin/bench_bitcoin -filter='.*MemPool.*' && \
    git checkout --quiet - && \
    git rebase -i --exec "cmake --build build && build/bin/bench_bitcoin -filter='.*MemPool.*'" "$BASE"

which includes the head of master in the benchmark results without inclusion in the rebase