From 494ffae2a5466a7a8a7ce092ddc1aa1f88f7cac7 Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Sat, 18 Jul 2026 16:35:07 +0200 Subject: [PATCH 01/12] Add benchmark harness skeleton: benchmarks/ + Google Benchmark + CTest smoke MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The vertical tracer bullet through the whole build+run+report path (NORTH_STAR.md §8.4): a new benchmarks/ tree, gated by AL_BUILD_BENCHMARKS and its own preset, off in the ordinary build and independent of AL_BUILD_TESTS. Google Benchmark is pinned via FetchContent (v1.9.5, no cluster module dependency). One benchmark drives the public C ABI end-to-end -- a single-leaf write-once-then-read against the in-RAM Memory backend on a tiny inline fixture, deliberately not the equilibrium seed yet -- emitting Google Benchmark JSON. A CTest smoke pair (mirroring tests/CMakeLists.txt's FIXTURES_SETUP/FIXTURES_REQUIRED shape) runs it with a one-iteration budget and asserts clean exit plus structurally parseable JSON, with no timing-value assertion. (cherry picked from commit f24b119a37b2ddffb823a30d1874dbbc8635bb3f) --- CMakeLists.txt | 11 +++ CMakePresets.json | 23 ++++++ benchmarks/CMakeLists.txt | 66 +++++++++++++++++ benchmarks/bench_write_read.cpp | 98 ++++++++++++++++++++++++++ benchmarks/validate_benchmark_json.cpp | 76 ++++++++++++++++++++ 5 files changed, 274 insertions(+) create mode 100644 benchmarks/CMakeLists.txt create mode 100644 benchmarks/bench_write_read.cpp create mode 100644 benchmarks/validate_benchmark_json.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index dcbb5780..8fd0b6d0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -37,6 +37,10 @@ option(AL_PYTHON_BINDINGS "Build Python bindings" OFF) option(AL_BUILD_TESTS "Build the C/C++ low-level ABI smoke tests (tests/)" OFF) +# Independent of AL_BUILD_TESTS: a benchmark "failing" means slower, not +# wrong, so it never gates the correctness build (NORTH_STAR.md §8.4). +option(AL_BUILD_BENCHMARKS "Build the Google Benchmark harness (benchmarks/)" OFF) + # The fixtures/al_contract_fixtures INTERFACE library itself is always # available (see below) so it is genuinely usable outside AL_BUILD_TESTS; this # only gates the throwaway demonstration executable (issue #51 acceptance @@ -246,6 +250,13 @@ if( AL_BUILD_TESTS ) add_subdirectory(tests) endif() +# Benchmark harness (opt-in, off in the ordinary build) +# ############################################################################## +if( AL_BUILD_BENCHMARKS ) + enable_testing() + add_subdirectory(benchmarks) +endif() + # Install # ############################################################################## diff --git a/CMakePresets.json b/CMakePresets.json index 96174b68..7e4b8a20 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -41,6 +41,18 @@ "AL_BUILD_MDSPLUS_MODELS": "ON", "CMAKE_BUILD_TYPE": "Release" } + }, + { + "name": "benchmarks", + "displayName": "Benchmark harness build (Release)", + "description": "Google Benchmark harness in benchmarks/ (issue #52); independent of AL_BUILD_TESTS and off in the ordinary build. A benchmark 'failing' means slower, not wrong, so this never gates the correctness suite.", + "generator": "Ninja", + "binaryDir": "${sourceDir}/build-benchmarks", + "cacheVariables": { + "AL_BUILD_BENCHMARKS": "ON", + "AL_PYTHON_BINDINGS": "OFF", + "CMAKE_BUILD_TYPE": "Release" + } } ], "buildPresets": [ @@ -55,6 +67,10 @@ "name": "mdsplus", "configurePreset": "mdsplus", "targets": [ "contract_tests" ] + }, + { + "name": "benchmarks", + "configurePreset": "benchmarks" } ], "testPresets": [ @@ -113,6 +129,13 @@ }, "output": { "outputOnFailure": true }, "execution": { "noTestsAction": "error" } + }, + { + "name": "benchmarks", + "displayName": "Benchmark harness smoke: proves build+run+report, asserts no timing value", + "configurePreset": "benchmarks", + "output": { "outputOnFailure": true }, + "execution": { "noTestsAction": "error" } } ] } diff --git a/benchmarks/CMakeLists.txt b/benchmarks/CMakeLists.txt new file mode 100644 index 00000000..96eeb60a --- /dev/null +++ b/benchmarks/CMakeLists.txt @@ -0,0 +1,66 @@ +project( al-benchmarks ) + +######################################################################################################################## +# Google Benchmark, pinned via FetchContent (NORTH_STAR.md §8.4). +# +# Unlike tests/contract's GoogleTest dependency, there is no find_package +# fallback here: the benchmark harness is a standalone tree gated by its own +# option (AL_BUILD_BENCHMARKS), and must not rely on a cluster module (e.g. +# the ITER SDCC `googlebenchmark` module) -- FetchContent keeps the pin +# identical on every machine the harness runs on. Pinned to the immutable +# commit for release v1.9.5 (a tag can be re-pointed). + +include(FetchContent) +FetchContent_Declare( + googlebenchmark + GIT_REPOSITORY https://github.com/google/benchmark.git + GIT_TAG 192ef10025eb2c4cdd392bc502f0c852196baa48 +) +set(BENCHMARK_ENABLE_TESTING OFF CACHE BOOL "" FORCE) +set(BENCHMARK_ENABLE_INSTALL OFF CACHE BOOL "" FORCE) +set(BENCHMARK_ENABLE_GTEST_TESTS OFF CACHE BOOL "" FORCE) +set(BENCHMARK_ENABLE_WERROR OFF CACHE BOOL "" FORCE) +FetchContent_MakeAvailable(googlebenchmark) + +######################################################################################################################## +# The benchmark executable. +# +# One benchmark drives the public C ABI (al_lowlevel.h) end-to-end: a +# single-leaf write-once-then-read on a tiny inline fixture -- deliberately NOT +# the equilibrium seed yet (that lands with the real workload issue). It runs +# against the in-RAM Memory backend, so the harness needs no filesystem or +# optional backend to smoke-prove the whole build+run+report path. + +add_executable( al_benchmarks bench_write_read.cpp ) +target_link_libraries( al_benchmarks PRIVATE al benchmark::benchmark ) +add_dependencies( al_benchmarks al ) +target_compile_features( al_benchmarks PRIVATE cxx_std_17 ) + +######################################################################################################################## +# CTest smoke entry. +# +# Proves the harness builds, links libal, drives the C ABI, and produces +# parseable Google Benchmark JSON -- it asserts none of the reported timing +# values (a benchmark "failing" means slower, not wrong; NORTH_STAR.md §8.4). +# --benchmark_min_time=1x caps every registered benchmark at exactly one +# iteration, so the smoke run stays fast regardless of what benchmarks are +# added later. +# +# Follows the FIXTURES_SETUP/FIXTURES_REQUIRED shape tests/CMakeLists.txt uses +# for its own smoke registration: one test produces the JSON file, a second +# frameworkless executable validates it. + +set( _smoke_json ${CMAKE_CURRENT_BINARY_DIR}/benchmark_smoke.json ) + +add_test( NAME benchmark_smoke_run + COMMAND al_benchmarks --benchmark_min_time=1x + --benchmark_out=${_smoke_json} + --benchmark_out_format=json ) +set_tests_properties( benchmark_smoke_run PROPERTIES FIXTURES_SETUP BENCHMARK_SMOKE_JSON ) + +add_executable( al_benchmark_json_check validate_benchmark_json.cpp ) +target_compile_features( al_benchmark_json_check PRIVATE cxx_std_17 ) + +add_test( NAME benchmark_smoke_validate + COMMAND al_benchmark_json_check ${_smoke_json} ) +set_tests_properties( benchmark_smoke_validate PROPERTIES FIXTURES_REQUIRED BENCHMARK_SMOKE_JSON ) diff --git a/benchmarks/bench_write_read.cpp b/benchmarks/bench_write_read.cpp new file mode 100644 index 00000000..55197427 --- /dev/null +++ b/benchmarks/bench_write_read.cpp @@ -0,0 +1,98 @@ +// This is the vertical tracer bullet through the whole build+run+report path: +// one benchmark, driving strictly the public C ABI (al_lowlevel.h), against a +// tiny inline fixture -- deliberately NOT the equilibrium seed yet, so this +// benchmark stays independent of that fixture prefactor. It exists to prove +// the harness itself (FetchContent pin, CTest smoke wiring, JSON output), +// not to produce a meaningful performance number; the real hot-path scenarios +// (whole-IDS read/write, AOS traversal, ...) land separately (NORTH_STAR.md §8.4). + +#include +#include + +#include + +#include +#include +#include + +namespace { + +// The core attaches no DD semantics to either string (DD paths are opaque +// strings through the whole C ABI), so an arbitrary IDS name and leaf path +// exercise the same code path a real IDS would. +constexpr const char* kIds = "magnetics"; +constexpr const char* kField = "ids_properties/homogeneous_time"; +constexpr int kValue = 42; + +void CheckOk(const al_status_t& status, const char* what) { + if (status.code != 0) { + throw std::runtime_error(std::string(what) + " failed (code=" + + std::to_string(status.code) + "): " + + status.message); + } +} + +// Builds a fresh in-RAM Memory-backend URI. The Memory backend never touches +// disk, so the harness has no filesystem dependency to smoke-validate the +// build+run+report path -- but the legacy-parameter URI builder always +// resolves `user` into a filesystem path before the backend even sees it +// (al_context.cpp's DataEntryContext::parseUri), and an empty/unknown user +// falls through to a getpwnam() lookup that throws. An absolute-looking +// string (never actually opened for Memory) sidesteps that lookup, the same +// trick tests/contract/al_contract.h's TempBase uses for on-disk backends. +// `pulse` is bumped by the caller on every iteration so FORCE_CREATE_PULSE +// never collides with a still-registered in-process pulse from a prior one. +std::string MemoryUri(int pulse) { + char* uri = nullptr; + CheckOk(al_build_uri_from_legacy_parameters( + MEMORY_BACKEND, pulse, /*run=*/0, /*user=*/"/al-benchmarks", + /*tokamak=*/"bench", /*version=*/"3", /*options=*/"", &uri), + "al_build_uri_from_legacy_parameters"); + std::string result(uri ? uri : ""); + free(uri); + return result; +} + +} // namespace + +// Single-leaf write-once-then-read: each iteration opens a fresh Memory-backend +// data entry, writes one INTEGER scalar, reads it back, and closes the pulse. +// The whole sequence is the timed region for this tracer-bullet benchmark; the +// finer open-once/timed-read-only split NORTH_STAR.md §8.4 calls for arrives +// with the real equilibrium read benchmark. +void BM_WriteReadRoundTrip(benchmark::State& state) { + int pulse = 0; + for (auto _ : state) { + const std::string uri = MemoryUri(pulse++); + + int pulse_ctx = -1; + CheckOk(al_begin_dataentry_action(uri.c_str(), FORCE_CREATE_PULSE, + &pulse_ctx), + "al_begin_dataentry_action"); + + int write_ctx = -1; + CheckOk(al_begin_global_action(pulse_ctx, kIds, "", WRITE_OP, &write_ctx), + "al_begin_global_action(write)"); + int value = kValue; + CheckOk(al_write_data(write_ctx, kField, "", &value, INTEGER_DATA, 0, + nullptr), + "al_write_data"); + CheckOk(al_end_action(write_ctx), "al_end_action(write)"); + + int read_ctx = -1; + CheckOk(al_begin_global_action(pulse_ctx, kIds, "", READ_OP, &read_ctx), + "al_begin_global_action(read)"); + int read_value = 0; + void* buf = &read_value; + int size[MAXDIM] = {0}; + CheckOk(al_read_data(read_ctx, kField, "", &buf, INTEGER_DATA, 0, size), + "al_read_data"); + CheckOk(al_end_action(read_ctx), "al_end_action(read)"); + benchmark::DoNotOptimize(read_value); + + CheckOk(al_close_pulse(pulse_ctx, CLOSE_PULSE), "al_close_pulse"); + } +} +BENCHMARK(BM_WriteReadRoundTrip); + +BENCHMARK_MAIN(); diff --git a/benchmarks/validate_benchmark_json.cpp b/benchmarks/validate_benchmark_json.cpp new file mode 100644 index 00000000..182f6676 --- /dev/null +++ b/benchmarks/validate_benchmark_json.cpp @@ -0,0 +1,76 @@ +// Frameworkless JSON-shape smoke check for the benchmark harness. +// +// The smoke test only needs proof that Google Benchmark emitted *parseable* +// JSON, never that any field or timing value is correct -- so this +// deliberately isn't a JSON library: a string/escape-aware brace-balance scan +// is enough to catch "crashed mid-write" or "not JSON at all" without adding +// a JSON dependency to a harness whose whole point is having none. + +#include +#include +#include +#include + +namespace { + +bool StructurallyBalanced(const std::string& text) { + int depth = 0; + bool in_string = false; + bool escaped = false; + for (char c : text) { + if (in_string) { + if (escaped) { + escaped = false; + } else if (c == '\\') { + escaped = true; + } else if (c == '"') { + in_string = false; + } + continue; + } + if (c == '"') { + in_string = true; + } else if (c == '{' || c == '[') { + ++depth; + } else if (c == '}' || c == ']') { + --depth; + if (depth < 0) return false; + } + } + return !in_string && depth == 0; +} + +} // namespace + +int main(int argc, char** argv) { + if (argc != 2) { + std::cerr << "usage: al_benchmark_json_check \n"; + return 1; + } + + std::ifstream file(argv[1], std::ios::binary); + if (!file) { + std::cerr << "al_benchmark_json_check: cannot open " << argv[1] << "\n"; + return 1; + } + std::ostringstream buffer; + buffer << file.rdbuf(); + const std::string content = buffer.str(); + + if (content.empty()) { + std::cerr << "al_benchmark_json_check: " << argv[1] << " is empty\n"; + return 1; + } + if (!StructurallyBalanced(content)) { + std::cerr << "al_benchmark_json_check: " << argv[1] + << " is not structurally balanced JSON\n"; + return 1; + } + if (content.find("\"benchmarks\"") == std::string::npos) { + std::cerr << "al_benchmark_json_check: " << argv[1] + << " has no top-level \"benchmarks\" key\n"; + return 1; + } + + return 0; +} From 400e54e0987ac9e33e0fb711cb447f4a7a14e2f6 Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Mon, 20 Jul 2026 10:21:08 +0200 Subject: [PATCH 02/12] Validate benchmark JSON with CMake parser (cherry picked from commit e2f2d19b0678b7bfc30e50a5724eb155ed640e11) --- benchmarks/CMakeLists.txt | 10 ++-- benchmarks/validate_benchmark_json.cmake | 27 +++++++++ benchmarks/validate_benchmark_json.cpp | 76 ------------------------ 3 files changed, 31 insertions(+), 82 deletions(-) create mode 100644 benchmarks/validate_benchmark_json.cmake delete mode 100644 benchmarks/validate_benchmark_json.cpp diff --git a/benchmarks/CMakeLists.txt b/benchmarks/CMakeLists.txt index 96eeb60a..82e4d38d 100644 --- a/benchmarks/CMakeLists.txt +++ b/benchmarks/CMakeLists.txt @@ -47,8 +47,8 @@ target_compile_features( al_benchmarks PRIVATE cxx_std_17 ) # added later. # # Follows the FIXTURES_SETUP/FIXTURES_REQUIRED shape tests/CMakeLists.txt uses -# for its own smoke registration: one test produces the JSON file, a second -# frameworkless executable validates it. +# for its own smoke registration: one test produces the JSON file, then CMake's +# built-in JSON parser validates it without adding another runtime dependency. set( _smoke_json ${CMAKE_CURRENT_BINARY_DIR}/benchmark_smoke.json ) @@ -58,9 +58,7 @@ add_test( NAME benchmark_smoke_run --benchmark_out_format=json ) set_tests_properties( benchmark_smoke_run PROPERTIES FIXTURES_SETUP BENCHMARK_SMOKE_JSON ) -add_executable( al_benchmark_json_check validate_benchmark_json.cpp ) -target_compile_features( al_benchmark_json_check PRIVATE cxx_std_17 ) - add_test( NAME benchmark_smoke_validate - COMMAND al_benchmark_json_check ${_smoke_json} ) + COMMAND ${CMAKE_COMMAND} -DINPUT=${_smoke_json} + -P ${CMAKE_CURRENT_SOURCE_DIR}/validate_benchmark_json.cmake ) set_tests_properties( benchmark_smoke_validate PROPERTIES FIXTURES_REQUIRED BENCHMARK_SMOKE_JSON ) diff --git a/benchmarks/validate_benchmark_json.cmake b/benchmarks/validate_benchmark_json.cmake new file mode 100644 index 00000000..9e06fe18 --- /dev/null +++ b/benchmarks/validate_benchmark_json.cmake @@ -0,0 +1,27 @@ +# Parse the Google Benchmark output rather than merely checking delimiter +# balance. CMake 3.21 is the repository minimum and provides string(JSON). + +if(NOT DEFINED INPUT) + message(FATAL_ERROR "benchmark JSON checker requires -DINPUT=") +endif() + +file(READ "${INPUT}" content) +if(content STREQUAL "") + message(FATAL_ERROR "benchmark JSON file is empty: ${INPUT}") +endif() + +string(JSON root_type ERROR_VARIABLE parse_error TYPE "${content}") +if(parse_error) + message(FATAL_ERROR "benchmark output is not parseable JSON: ${parse_error}") +endif() +if(NOT root_type STREQUAL "OBJECT") + message(FATAL_ERROR "benchmark JSON root is not an object") +endif() + +string(JSON benchmarks_type ERROR_VARIABLE benchmarks_error TYPE "${content}" benchmarks) +if(benchmarks_error) + message(FATAL_ERROR "benchmark JSON has no top-level 'benchmarks' key: ${benchmarks_error}") +endif() +if(NOT benchmarks_type STREQUAL "ARRAY") + message(FATAL_ERROR "benchmark JSON 'benchmarks' key is not an array") +endif() diff --git a/benchmarks/validate_benchmark_json.cpp b/benchmarks/validate_benchmark_json.cpp deleted file mode 100644 index 182f6676..00000000 --- a/benchmarks/validate_benchmark_json.cpp +++ /dev/null @@ -1,76 +0,0 @@ -// Frameworkless JSON-shape smoke check for the benchmark harness. -// -// The smoke test only needs proof that Google Benchmark emitted *parseable* -// JSON, never that any field or timing value is correct -- so this -// deliberately isn't a JSON library: a string/escape-aware brace-balance scan -// is enough to catch "crashed mid-write" or "not JSON at all" without adding -// a JSON dependency to a harness whose whole point is having none. - -#include -#include -#include -#include - -namespace { - -bool StructurallyBalanced(const std::string& text) { - int depth = 0; - bool in_string = false; - bool escaped = false; - for (char c : text) { - if (in_string) { - if (escaped) { - escaped = false; - } else if (c == '\\') { - escaped = true; - } else if (c == '"') { - in_string = false; - } - continue; - } - if (c == '"') { - in_string = true; - } else if (c == '{' || c == '[') { - ++depth; - } else if (c == '}' || c == ']') { - --depth; - if (depth < 0) return false; - } - } - return !in_string && depth == 0; -} - -} // namespace - -int main(int argc, char** argv) { - if (argc != 2) { - std::cerr << "usage: al_benchmark_json_check \n"; - return 1; - } - - std::ifstream file(argv[1], std::ios::binary); - if (!file) { - std::cerr << "al_benchmark_json_check: cannot open " << argv[1] << "\n"; - return 1; - } - std::ostringstream buffer; - buffer << file.rdbuf(); - const std::string content = buffer.str(); - - if (content.empty()) { - std::cerr << "al_benchmark_json_check: " << argv[1] << " is empty\n"; - return 1; - } - if (!StructurallyBalanced(content)) { - std::cerr << "al_benchmark_json_check: " << argv[1] - << " is not structurally balanced JSON\n"; - return 1; - } - if (content.find("\"benchmarks\"") == std::string::npos) { - std::cerr << "al_benchmark_json_check: " << argv[1] - << " has no top-level \"benchmarks\" key\n"; - return 1; - } - - return 0; -} From 2bbe69518a38ee84e3e5605c6964d73520737ace Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Mon, 20 Jul 2026 10:33:59 +0200 Subject: [PATCH 03/12] Add whole-IDS equilibrium read benchmark on HDF5 (issue #53) Reuses equilibrium_seed.h (issue #51) verbatim as the workload, driven strictly through the public C ABI. Write + open happen once in untimed setup; the timed region is exactly equilibrium_seed::read_back's begin_global_action(READ) -> traverse every leaf -> end_action, repeated across 20 repetitions so Google Benchmark's JSON reports a distribution (mean/median/stddev/iterations), not a bare mean. Splits BENCHMARK_MAIN() into its own bench_main.cpp now that a second bench_*.cpp exists, and factors the shared CheckOk status-checking helper into bench_common.h so it isn't duplicated across benchmark files. (cherry picked from commit 144206ec6f83745d1b02a91e2c93f1ed70cb94a0) --- benchmarks/CMakeLists.txt | 30 ++++++-- benchmarks/bench_common.h | 25 +++++++ benchmarks/bench_equilibrium_read.cpp | 104 ++++++++++++++++++++++++++ benchmarks/bench_main.cpp | 7 ++ benchmarks/bench_write_read.cpp | 15 +--- 5 files changed, 163 insertions(+), 18 deletions(-) create mode 100644 benchmarks/bench_common.h create mode 100644 benchmarks/bench_equilibrium_read.cpp create mode 100644 benchmarks/bench_main.cpp diff --git a/benchmarks/CMakeLists.txt b/benchmarks/CMakeLists.txt index 82e4d38d..201326dd 100644 --- a/benchmarks/CMakeLists.txt +++ b/benchmarks/CMakeLists.txt @@ -25,17 +25,33 @@ FetchContent_MakeAvailable(googlebenchmark) ######################################################################################################################## # The benchmark executable. # -# One benchmark drives the public C ABI (al_lowlevel.h) end-to-end: a -# single-leaf write-once-then-read on a tiny inline fixture -- deliberately NOT -# the equilibrium seed yet (that lands with the real workload issue). It runs -# against the in-RAM Memory backend, so the harness needs no filesystem or -# optional backend to smoke-prove the whole build+run+report path. +# bench_write_read.cpp is the original tracer bullet: a single-leaf +# write-once-then-read on a tiny inline fixture, against the in-RAM Memory +# backend, needing no filesystem or optional backend. +# +# bench_equilibrium_read.cpp is the real P1 headline workload (issue #53): +# whole-IDS equilibrium read on the HDF5 backend, reusing equilibrium_seed.h +# (issue #51) via the al_contract_fixtures INTERFACE library below -- hence +# needs the filesystem (std::filesystem temp dirs) that Memory-only +# bench_write_read.cpp does not. +# +# bench_main.cpp holds the one BENCHMARK_MAIN() shared by every bench_*.cpp +# TU in this executable. -add_executable( al_benchmarks bench_write_read.cpp ) -target_link_libraries( al_benchmarks PRIVATE al benchmark::benchmark ) +add_executable( al_benchmarks + bench_write_read.cpp + bench_equilibrium_read.cpp + bench_main.cpp ) +target_link_libraries( al_benchmarks PRIVATE al_contract_fixtures benchmark::benchmark ) add_dependencies( al_benchmarks al ) target_compile_features( al_benchmarks PRIVATE cxx_std_17 ) +if(NOT WIN32 AND NOT APPLE) + # stdc++fs is only needed on Linux with older GCC (std::filesystem use in + # bench_equilibrium_read.cpp), mirroring fixtures/CMakeLists.txt's smoke target. + target_link_libraries( al_benchmarks PRIVATE stdc++fs ) +endif() + ######################################################################################################################## # CTest smoke entry. # diff --git a/benchmarks/bench_common.h b/benchmarks/bench_common.h new file mode 100644 index 00000000..7b248a3a --- /dev/null +++ b/benchmarks/bench_common.h @@ -0,0 +1,25 @@ +// Shared by every bench_*.cpp TU in al_benchmarks: the one status-checking +// helper each benchmark uses to turn a failing al_status_t into a thrown +// exception (Google Benchmark reports the failure instead of silently timing +// garbage). Header-only so it carries no extra link surface. +#ifndef AL_BENCHMARKS_COMMON_H +#define AL_BENCHMARKS_COMMON_H + +#include + +#include +#include + +namespace al_bench { + +inline void CheckOk(const al_status_t& status, const char* what) { + if (status.code != 0) { + throw std::runtime_error(std::string(what) + " failed (code=" + + std::to_string(status.code) + "): " + + status.message); + } +} + +} // namespace al_bench + +#endif // AL_BENCHMARKS_COMMON_H diff --git a/benchmarks/bench_equilibrium_read.cpp b/benchmarks/bench_equilibrium_read.cpp new file mode 100644 index 00000000..f245ee6c --- /dev/null +++ b/benchmarks/bench_equilibrium_read.cpp @@ -0,0 +1,104 @@ +// The headline §7 P1 metric (NORTH_STAR.md §8.4 "whole-IDS get"): a whole-IDS +// read of the realistic equilibrium fixture through the HDF5 backend, driven +// strictly at the public C ABI (al_lowlevel.h). Reuses equilibrium_seed.h +// verbatim (issue #51) as the workload -- no bespoke fixture -- so the +// benchmarked shape never drifts from what the contract suite already pins. +// +// Timed region = one operation (issue #53, PRD #50 user story 6): the +// reference equilibrium is written and the data entry opened once in untimed +// setup; only equilibrium_seed::read_back's begin_global_action(READ) -> +// traverse every leaf -> end_action is inside the `for (auto _ : state)` loop. +// Backend is a sub-axis of the whole-IDS-read operation (PRD #50 "structure: +// scenario-primary, backend as a sub-axis"), so the Memory control (issue #54) +// lands as a second BENCHMARK() registration reusing this same function shape. + +#include "bench_common.h" +#include "equilibrium_seed.h" + +#include +#include + +#include + +#include +#include +#include + +#if defined(_WIN32) +#include +#define AL_BENCH_GETPID _getpid +#else +#include +#define AL_BENCH_GETPID getpid +#endif + +namespace { + +using al_bench::CheckOk; + +// The HDF5 backend is on-disk (unlike Memory in bench_write_read.cpp), so +// FORCE_CREATE_PULSE needs the legacy //// tree +// pre-created -- mirrors tests/contract/al_contract.h's TempBase, reimplemented +// here without a GoogleTest dependency (this tree links al_contract_fixtures, +// not al_contract.h). RAII like TempBase: the directory is created in the +// constructor and always removed in the destructor, including when a +// CheckOk() throws mid-setup, so a failing run never leaks the temp dir. +class TempHdf5Dir { +public: + TempHdf5Dir() + : path_(std::filesystem::temp_directory_path() / + ("al_benchmarks_hdf5_" + std::to_string(AL_BENCH_GETPID()))) { + std::error_code ec; + std::filesystem::remove_all(path_, ec); + std::filesystem::create_directories(path_ / "bench" / "3" / "1" / "0", ec); + } + ~TempHdf5Dir() { + std::error_code ec; + std::filesystem::remove_all(path_, ec); + } + TempHdf5Dir(const TempHdf5Dir&) = delete; + TempHdf5Dir& operator=(const TempHdf5Dir&) = delete; + + std::string uri() const { + char* uri = nullptr; + CheckOk(al_build_uri_from_legacy_parameters( + HDF5_BACKEND, /*pulse=*/1, /*run=*/0, path_.string().c_str(), + /*tokamak=*/"bench", /*version=*/"3", /*options=*/"", &uri), + "al_build_uri_from_legacy_parameters"); + std::string result(uri ? uri : ""); + free(uri); + return result; + } + +private: + std::filesystem::path path_; +}; + +} // namespace + +void BM_EquilibriumWholeIdsRead_HDF5(benchmark::State& state) { + TempHdf5Dir dir; + const std::string uri = dir.uri(); + + // --- untimed setup: open once, write the reference equilibrium once ------ + int pulse_ctx = -1; + CheckOk(al_begin_dataentry_action(uri.c_str(), FORCE_CREATE_PULSE, &pulse_ctx), + "al_begin_dataentry_action"); + CheckOk(equilibrium_seed::write(pulse_ctx), "equilibrium_seed::write"); + + // --- timed region: begin_global_action(READ) -> every leaf -> end_action - + std::vector records; + for (auto _ : state) { + CheckOk(equilibrium_seed::read_back(pulse_ctx, &records), + "equilibrium_seed::read_back"); + benchmark::DoNotOptimize(records); + } + + // --- untimed teardown ------------------------------------------------------ + CheckOk(al_close_pulse(pulse_ctx, CLOSE_PULSE), "al_close_pulse"); + // dir's destructor removes the temp directory on every exit path. +} +// Repetitions(N) makes Google Benchmark report the distribution (median, +// mean, stddev, iteration count) across N independent setup/read/teardown +// cycles, not a bare single-shot mean (issue #53, PRD #50 user story 8). +BENCHMARK(BM_EquilibriumWholeIdsRead_HDF5)->Repetitions(20); diff --git a/benchmarks/bench_main.cpp b/benchmarks/bench_main.cpp new file mode 100644 index 00000000..05bc67da --- /dev/null +++ b/benchmarks/bench_main.cpp @@ -0,0 +1,7 @@ +// Shared Google Benchmark entry point. Every bench_*.cpp in al_benchmarks only +// registers benchmarks via BENCHMARK(...); exactly one translation unit may +// define main(), so it lives here rather than in whichever bench_*.cpp file +// happened to have it first. +#include + +BENCHMARK_MAIN(); diff --git a/benchmarks/bench_write_read.cpp b/benchmarks/bench_write_read.cpp index 55197427..f204c704 100644 --- a/benchmarks/bench_write_read.cpp +++ b/benchmarks/bench_write_read.cpp @@ -6,17 +6,20 @@ // not to produce a meaningful performance number; the real hot-path scenarios // (whole-IDS read/write, AOS traversal, ...) land separately (NORTH_STAR.md §8.4). +#include "bench_common.h" + #include #include #include #include -#include #include namespace { +using al_bench::CheckOk; + // The core attaches no DD semantics to either string (DD paths are opaque // strings through the whole C ABI), so an arbitrary IDS name and leaf path // exercise the same code path a real IDS would. @@ -24,14 +27,6 @@ constexpr const char* kIds = "magnetics"; constexpr const char* kField = "ids_properties/homogeneous_time"; constexpr int kValue = 42; -void CheckOk(const al_status_t& status, const char* what) { - if (status.code != 0) { - throw std::runtime_error(std::string(what) + " failed (code=" + - std::to_string(status.code) + "): " + - status.message); - } -} - // Builds a fresh in-RAM Memory-backend URI. The Memory backend never touches // disk, so the harness has no filesystem dependency to smoke-validate the // build+run+report path -- but the legacy-parameter URI builder always @@ -94,5 +89,3 @@ void BM_WriteReadRoundTrip(benchmark::State& state) { } } BENCHMARK(BM_WriteReadRoundTrip); - -BENCHMARK_MAIN(); From 982e5cc6f692d896e18ffb1e02d8ae4bd99398ed Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Mon, 20 Jul 2026 11:06:00 +0200 Subject: [PATCH 04/12] Share legacy pulse storage between tests and benchmarks (cherry picked from commit 7246824127a3339e6864f91238eb434a8ebb2afc) --- benchmarks/bench_equilibrium_read.cpp | 67 +++++++---------------- fixtures/CMakeLists.txt | 5 +- fixtures/al_contract_legacy_storage.h | 76 +++++++++++++++++++++++++++ tests/contract/al_contract.h | 69 ++---------------------- 4 files changed, 102 insertions(+), 115 deletions(-) create mode 100644 fixtures/al_contract_legacy_storage.h diff --git a/benchmarks/bench_equilibrium_read.cpp b/benchmarks/bench_equilibrium_read.cpp index f245ee6c..2508cb81 100644 --- a/benchmarks/bench_equilibrium_read.cpp +++ b/benchmarks/bench_equilibrium_read.cpp @@ -13,6 +13,7 @@ // lands as a second BENCHMARK() registration reusing this same function shape. #include "bench_common.h" +#include "al_contract_legacy_storage.h" #include "equilibrium_seed.h" #include @@ -21,64 +22,32 @@ #include #include -#include #include -#if defined(_WIN32) -#include -#define AL_BENCH_GETPID _getpid -#else -#include -#define AL_BENCH_GETPID getpid -#endif - namespace { using al_bench::CheckOk; -// The HDF5 backend is on-disk (unlike Memory in bench_write_read.cpp), so -// FORCE_CREATE_PULSE needs the legacy //// tree -// pre-created -- mirrors tests/contract/al_contract.h's TempBase, reimplemented -// here without a GoogleTest dependency (this tree links al_contract_fixtures, -// not al_contract.h). RAII like TempBase: the directory is created in the -// constructor and always removed in the destructor, including when a -// CheckOk() throws mid-setup, so a failing run never leaks the temp dir. -class TempHdf5Dir { -public: - TempHdf5Dir() - : path_(std::filesystem::temp_directory_path() / - ("al_benchmarks_hdf5_" + std::to_string(AL_BENCH_GETPID()))) { - std::error_code ec; - std::filesystem::remove_all(path_, ec); - std::filesystem::create_directories(path_ / "bench" / "3" / "1" / "0", ec); - } - ~TempHdf5Dir() { - std::error_code ec; - std::filesystem::remove_all(path_, ec); - } - TempHdf5Dir(const TempHdf5Dir&) = delete; - TempHdf5Dir& operator=(const TempHdf5Dir&) = delete; - - std::string uri() const { - char* uri = nullptr; - CheckOk(al_build_uri_from_legacy_parameters( - HDF5_BACKEND, /*pulse=*/1, /*run=*/0, path_.string().c_str(), - /*tokamak=*/"bench", /*version=*/"3", /*options=*/"", &uri), - "al_build_uri_from_legacy_parameters"); - std::string result(uri ? uri : ""); - free(uri); - return result; - } - -private: - std::filesystem::path path_; -}; +std::string Hdf5Uri(const al_contract::LegacyPulseDirectory& base, + const al_contract::PulseId& pulse) { + char* uri = nullptr; + CheckOk(al_build_uri_from_legacy_parameters( + HDF5_BACKEND, pulse.pulse, pulse.run, base.str().c_str(), + pulse.database.c_str(), pulse.version.c_str(), /*options=*/"", &uri), + "al_build_uri_from_legacy_parameters"); + std::string result(uri ? uri : ""); + free(uri); + return result; +} } // namespace void BM_EquilibriumWholeIdsRead_HDF5(benchmark::State& state) { - TempHdf5Dir dir; - const std::string uri = dir.uri(); + const al_contract::PulseId pulse{/*database=*/"bench", /*version=*/"3", + /*pulse=*/1, /*run=*/0}; + al_contract::LegacyPulseDirectory base; + base.make_legacy_tree(pulse); + const std::string uri = Hdf5Uri(base, pulse); // --- untimed setup: open once, write the reference equilibrium once ------ int pulse_ctx = -1; @@ -96,7 +65,7 @@ void BM_EquilibriumWholeIdsRead_HDF5(benchmark::State& state) { // --- untimed teardown ------------------------------------------------------ CheckOk(al_close_pulse(pulse_ctx, CLOSE_PULSE), "al_close_pulse"); - // dir's destructor removes the temp directory on every exit path. + // base's destructor removes the temp directory on every exit path. } // Repetitions(N) makes Google Benchmark report the distribution (median, // mean, stddev, iteration count) across N independent setup/read/teardown diff --git a/fixtures/CMakeLists.txt b/fixtures/CMakeLists.txt index c674ed3d..fa4f92eb 100644 --- a/fixtures/CMakeLists.txt +++ b/fixtures/CMakeLists.txt @@ -1,8 +1,9 @@ ######################################################################################################################## # Shared contract fixtures (issue #51). # -# The realistic equilibrium seed (equilibrium_seed.h) and the generic typed -# ABI round-trip helpers it needs (al_contract_abi.h), factored out of +# The realistic equilibrium seed (equilibrium_seed.h), legacy on-disk storage +# helper (al_contract_legacy_storage.h), and generic typed ABI round-trip +# helpers (al_contract_abi.h), factored out of # tests/contract/ so a build target outside the test tree -- the future # benchmarks/ tree (PRD #50, issues #52/#53) -- can link the exact same # workload the contract suite pins. No fixture drift between what is tested diff --git a/fixtures/al_contract_legacy_storage.h b/fixtures/al_contract_legacy_storage.h new file mode 100644 index 00000000..76e97718 --- /dev/null +++ b/fixtures/al_contract_legacy_storage.h @@ -0,0 +1,76 @@ +// Shared filesystem support for contract fixtures and benchmarks that exercise +// the legacy data-entry URI layout. This header deliberately has no GoogleTest +// dependency: callers choose how to report URI or ABI failures. +#ifndef AL_CONTRACT_LEGACY_STORAGE_H +#define AL_CONTRACT_LEGACY_STORAGE_H + +#include +#include +#include + +#if defined(_WIN32) +#include +#else +#include +#endif + +namespace al_contract { + +// The legacy address of a pulse: (database, version, pulse, run). These four +// values always travel together — into the URI and into the on-disk tree path. +struct PulseId { + std::string database; + std::string version; + int pulse; + int run; +}; + +// Owns a unique temporary base directory and prepares the legacy pulse layout +// required by on-disk backends using FORCE_CREATE_PULSE. +class LegacyPulseDirectory { +public: + LegacyPulseDirectory() { + static std::atomic counter{0}; + const unsigned n = counter.fetch_add(1); + path_ = std::filesystem::temp_directory_path() / + ("al_contract_" + std::to_string(process_id()) + "_" + + std::to_string(n)); + std::error_code ec; + std::filesystem::remove_all(path_, ec); + std::filesystem::create_directories(path_, ec); + } + + ~LegacyPulseDirectory() { + std::error_code ec; + std::filesystem::remove_all(path_, ec); + } + + LegacyPulseDirectory(const LegacyPulseDirectory&) = delete; + LegacyPulseDirectory& operator=(const LegacyPulseDirectory&) = delete; + + const std::filesystem::path& path() const { return path_; } + std::string str() const { return path_.string(); } + + void make_legacy_tree(const PulseId& id) const { + std::error_code ec; + std::filesystem::create_directories( + path_ / id.database / id.version / std::to_string(id.pulse) / + std::to_string(id.run), + ec); + } + +private: + static int process_id() { +#if defined(_WIN32) + return _getpid(); +#else + return getpid(); +#endif + } + + std::filesystem::path path_; +}; + +} // namespace al_contract + +#endif // AL_CONTRACT_LEGACY_STORAGE_H diff --git a/tests/contract/al_contract.h b/tests/contract/al_contract.h index 616a5b55..85a0eb8c 100644 --- a/tests/contract/al_contract.h +++ b/tests/contract/al_contract.h @@ -13,34 +13,28 @@ #define AL_CONTRACT_H #include "al_contract_abi.h" +#include "al_contract_legacy_storage.h" #include #include #include -#include #include #include #include #include -#include #include #include #include -#if defined(_WIN32) -#include -#define AL_CONTRACT_GETPID _getpid -#else +#if !defined(_WIN32) #if defined(AL_CONTRACT_HAVE_UDA) #include #include #include #include #endif -#include -#define AL_CONTRACT_GETPID getpid #endif namespace al_contract { @@ -208,62 +202,9 @@ struct BackendCase { inline void PrintTo(const BackendCase& b, std::ostream* os) { *os << b.name; } // --------------------------------------------------------------------------- -// The legacy address of a pulse: (database, version, pulse, run). These four -// always travel together — into the URI and into the on-disk tree path — so -// they are one value, not four loose args. -// --------------------------------------------------------------------------- -struct PulseId { - std::string database; - std::string version; - int pulse; - int run; -}; - -// --------------------------------------------------------------------------- -// Unique temp base directory, RAII-cleaned. -// --------------------------------------------------------------------------- -// The legacy pulse path is ////; the core -// derives it when the base dir is supplied as the "user" field of the legacy -// URI (exactly how tests/CMakeLists.txt drives the existing smoke tests). Each -// fixture instance gets its own base dir so parallel ctest runs never collide: -// name = /al_contract__. -class TempBase { -public: - TempBase() { - static std::atomic counter{0}; - namespace fs = std::filesystem; - const unsigned n = counter.fetch_add(1); - path_ = fs::temp_directory_path() / - ("al_contract_" + std::to_string(AL_CONTRACT_GETPID()) + - "_" + std::to_string(n)); - std::error_code ec; - fs::remove_all(path_, ec); - fs::create_directories(path_, ec); - } - - ~TempBase() { - std::error_code ec; - std::filesystem::remove_all(path_, ec); - } - - TempBase(const TempBase&) = delete; - TempBase& operator=(const TempBase&) = delete; - - std::string str() const { return path_.string(); } - - // Pre-create the legacy pulse subtree an on-disk backend expects for - // FORCE_CREATE_PULSE. - void make_legacy_tree(const PulseId& id) const { - std::error_code ec; - std::filesystem::create_directories( - path_ / id.database / id.version / std::to_string(id.pulse) / - std::to_string(id.run), - ec); - } - -private: - std::filesystem::path path_; -}; +// Compatibility name for the existing contract tests. The implementation is +// shared with benchmarks through fixtures/al_contract_legacy_storage.h. +using TempBase = LegacyPulseDirectory; // --------------------------------------------------------------------------- // URI construction through the legacy-parameter ABI. From 6d491d4e558256e1990342babc6f6816cd4b4086 Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Mon, 20 Jul 2026 11:54:42 +0200 Subject: [PATCH 05/12] Add Memory backend as zero-I/O control for whole-IDS read (issue #54) Refactors the HDF5-only equilibrium whole-IDS read benchmark (#53) into one shared BM_EquilibriumWholeIdsRead body parameterized by a BenchBackend enum, registered via BENCHMARK_CAPTURE for both HDF5 and the in-RAM Memory backend. Memory isolates per-node traversal/CPU cost from HDF5's I/O cost, and adding it required only branching the URI-building step -- validating that backend is a cheap sub-axis of the operation rather than a copy-pasted scenario. (cherry picked from commit af0fb25dbb2e73c909fe95f777377fc6ed73eef4) --- benchmarks/CMakeLists.txt | 8 ++-- benchmarks/bench_equilibrium_read.cpp | 69 ++++++++++++++++++++------- 2 files changed, 55 insertions(+), 22 deletions(-) diff --git a/benchmarks/CMakeLists.txt b/benchmarks/CMakeLists.txt index 201326dd..4d7cb55f 100644 --- a/benchmarks/CMakeLists.txt +++ b/benchmarks/CMakeLists.txt @@ -30,10 +30,10 @@ FetchContent_MakeAvailable(googlebenchmark) # backend, needing no filesystem or optional backend. # # bench_equilibrium_read.cpp is the real P1 headline workload (issue #53): -# whole-IDS equilibrium read on the HDF5 backend, reusing equilibrium_seed.h -# (issue #51) via the al_contract_fixtures INTERFACE library below -- hence -# needs the filesystem (std::filesystem temp dirs) that Memory-only -# bench_write_read.cpp does not. +# whole-IDS equilibrium read, reusing equilibrium_seed.h (issue #51) via the +# al_contract_fixtures INTERFACE library below. It registers the same +# operation against both HDF5 (needs the filesystem -- std::filesystem temp +# dirs) and, as a zero-I/O control (issue #54), the in-RAM Memory backend. # # bench_main.cpp holds the one BENCHMARK_MAIN() shared by every bench_*.cpp # TU in this executable. diff --git a/benchmarks/bench_equilibrium_read.cpp b/benchmarks/bench_equilibrium_read.cpp index 2508cb81..b7a75a65 100644 --- a/benchmarks/bench_equilibrium_read.cpp +++ b/benchmarks/bench_equilibrium_read.cpp @@ -1,16 +1,21 @@ // The headline §7 P1 metric (NORTH_STAR.md §8.4 "whole-IDS get"): a whole-IDS -// read of the realistic equilibrium fixture through the HDF5 backend, driven -// strictly at the public C ABI (al_lowlevel.h). Reuses equilibrium_seed.h -// verbatim (issue #51) as the workload -- no bespoke fixture -- so the -// benchmarked shape never drifts from what the contract suite already pins. +// read of the realistic equilibrium fixture, driven strictly at the public C +// ABI (al_lowlevel.h). Reuses equilibrium_seed.h verbatim (issue #51) as the +// workload -- no bespoke fixture -- so the benchmarked shape never drifts +// from what the contract suite already pins. // // Timed region = one operation (issue #53, PRD #50 user story 6): the // reference equilibrium is written and the data entry opened once in untimed // setup; only equilibrium_seed::read_back's begin_global_action(READ) -> // traverse every leaf -> end_action is inside the `for (auto _ : state)` loop. +// // Backend is a sub-axis of the whole-IDS-read operation (PRD #50 "structure: -// scenario-primary, backend as a sub-axis"), so the Memory control (issue #54) -// lands as a second BENCHMARK() registration reusing this same function shape. +// scenario-primary, backend as a sub-axis"): BM_EquilibriumWholeIdsRead below +// is the one operation body, and HDF5 (issue #53) / Memory (issue #54) are +// two BENCHMARK_CAPTURE registrations of it, differing only in which backend +// id the URI is built for. Memory is a zero-I/O control -- it never touches +// disk -- so subtracting its number from HDF5's decomposes the total into +// storage cost vs per-node traversal/CPU cost. #include "bench_common.h" #include "al_contract_legacy_storage.h" @@ -22,19 +27,36 @@ #include #include +#include #include namespace { using al_bench::CheckOk; -std::string Hdf5Uri(const al_contract::LegacyPulseDirectory& base, - const al_contract::PulseId& pulse) { +enum class BenchBackend { kHdf5, kMemory }; + +// HDF5 resolves FORCE_CREATE_PULSE against a real on-disk legacy tree, so it +// needs `base` alive for the URI to mean anything. Memory never touches disk +// (bench_write_read.cpp's MemoryUri established the pattern): an arbitrary +// absolute-looking `user` sidesteps the legacy URI builder's getpwnam() +// fallback for an empty/unknown user, without paying for a real temp dir. +std::string BuildUri(BenchBackend backend, const al_contract::PulseId& pulse, + const al_contract::LegacyPulseDirectory* base) { char* uri = nullptr; - CheckOk(al_build_uri_from_legacy_parameters( - HDF5_BACKEND, pulse.pulse, pulse.run, base.str().c_str(), - pulse.database.c_str(), pulse.version.c_str(), /*options=*/"", &uri), - "al_build_uri_from_legacy_parameters"); + if (backend == BenchBackend::kHdf5) { + CheckOk(al_build_uri_from_legacy_parameters( + HDF5_BACKEND, pulse.pulse, pulse.run, base->str().c_str(), + pulse.database.c_str(), pulse.version.c_str(), + /*options=*/"", &uri), + "al_build_uri_from_legacy_parameters"); + } else { + CheckOk(al_build_uri_from_legacy_parameters( + MEMORY_BACKEND, pulse.pulse, pulse.run, "/al-benchmarks", + pulse.database.c_str(), pulse.version.c_str(), + /*options=*/"", &uri), + "al_build_uri_from_legacy_parameters"); + } std::string result(uri ? uri : ""); free(uri); return result; @@ -42,14 +64,22 @@ std::string Hdf5Uri(const al_contract::LegacyPulseDirectory& base, } // namespace -void BM_EquilibriumWholeIdsRead_HDF5(benchmark::State& state) { +void BM_EquilibriumWholeIdsRead(benchmark::State& state, BenchBackend backend) { const al_contract::PulseId pulse{/*database=*/"bench", /*version=*/"3", /*pulse=*/1, /*run=*/0}; - al_contract::LegacyPulseDirectory base; - base.make_legacy_tree(pulse); - const std::string uri = Hdf5Uri(base, pulse); + std::optional base; + if (backend == BenchBackend::kHdf5) { + base.emplace(); + base->make_legacy_tree(pulse); + } + const std::string uri = BuildUri(backend, pulse, base ? &*base : nullptr); // --- untimed setup: open once, write the reference equilibrium once ------ + // Memory keeps the fixed pulse identity across repetitions unlike + // bench_write_read.cpp's per-iteration bump: MemoryBackend::closePulse is a + // no-op (src/memory_backend.h), so correctness across the 20 Repetitions + // rests on FORCE_CREATE_PULSE clearing the reused entry's idsMap on reopen, + // not on pulse identities never colliding. int pulse_ctx = -1; CheckOk(al_begin_dataentry_action(uri.c_str(), FORCE_CREATE_PULSE, &pulse_ctx), "al_begin_dataentry_action"); @@ -65,9 +95,12 @@ void BM_EquilibriumWholeIdsRead_HDF5(benchmark::State& state) { // --- untimed teardown ------------------------------------------------------ CheckOk(al_close_pulse(pulse_ctx, CLOSE_PULSE), "al_close_pulse"); - // base's destructor removes the temp directory on every exit path. + // base's destructor (HDF5 only) removes the temp directory on every exit path. } // Repetitions(N) makes Google Benchmark report the distribution (median, // mean, stddev, iteration count) across N independent setup/read/teardown // cycles, not a bare single-shot mean (issue #53, PRD #50 user story 8). -BENCHMARK(BM_EquilibriumWholeIdsRead_HDF5)->Repetitions(20); +BENCHMARK_CAPTURE(BM_EquilibriumWholeIdsRead, HDF5, BenchBackend::kHdf5) + ->Repetitions(20); +BENCHMARK_CAPTURE(BM_EquilibriumWholeIdsRead, Memory, BenchBackend::kMemory) + ->Repetitions(20); From f1f741490573158849f9303cf1459720d5d57c38 Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Mon, 20 Jul 2026 11:56:45 +0200 Subject: [PATCH 06/12] Add baseline capture tooling: metadata + committed-artifact convention (issue #55) capture_baseline.py wraps al_benchmarks, tags the resulting JSON with comparability metadata (OS, CPU model, compiler+flags, HDF5 version, filesystem, SLURM partition, commit SHA), and writes it to benchmarks/baselines//.json. BASELINE_CAPTURE.md documents the laptop-smoke and ITER-SDCC-cluster procedures; the SLURM partition is always a runtime parameter, never hard-coded. (cherry picked from commit 4a20b8110efa5b11018a13eb7fafdf552bc9f723) --- benchmarks/BASELINE_CAPTURE.md | 107 +++++++++++ benchmarks/baselines/README.md | 66 +++++++ benchmarks/capture_baseline.py | 335 +++++++++++++++++++++++++++++++++ 3 files changed, 508 insertions(+) create mode 100644 benchmarks/BASELINE_CAPTURE.md create mode 100644 benchmarks/baselines/README.md create mode 100755 benchmarks/capture_baseline.py diff --git a/benchmarks/BASELINE_CAPTURE.md b/benchmarks/BASELINE_CAPTURE.md new file mode 100644 index 00000000..4b93a896 --- /dev/null +++ b/benchmarks/BASELINE_CAPTURE.md @@ -0,0 +1,107 @@ +# Capturing a baseline + +This documents the repeatable procedure behind `capture_baseline.py` (issue +#55): turning a run of `al_benchmarks` into a metadata-tagged, committable +JSON artifact under [`benchmarks/baselines/`](baselines/README.md). + +The script wraps whatever benchmarks are registered — it does not know or +care which scenarios exist, so this procedure does not change as +`bench_*.cpp` grows beyond the current whole-IDS-read operation. + +## Laptop (development / smoke — throwaway numbers) + +Any machine, any state of the working tree. These numbers exist to prove the +capture flow works, never to be compared against anything. + +```sh +cmake --preset benchmarks +cmake --build --preset benchmarks + +python3 benchmarks/capture_baseline.py \ + --binary build-benchmarks/benchmarks/al_benchmarks \ + --machine-id "laptop-$(hostname -s)" \ + -- --benchmark_min_time=1x +``` + +`--benchmark_min_time=1x` caps every registered benchmark at one iteration — +the same flag the CTest smoke check (`benchmark_smoke_run`) uses — so this +finishes in well under a second regardless of how many scenarios exist. +Drop it to get real repetition counts (slower, and still not a measurement +of record without the cluster's isolation). + +The script prints the metadata it captured (OS, CPU, compiler, HDF5 version, +filesystem, commit SHA/dirty flag, `slurm_partition: null`) and the path it +wrote to. On a laptop `commit_dirty` will often be `true` — that's expected +and is exactly why this file is a smoke artifact, not a baseline of record. + +## ITER SDCC cluster (measurement of record) + +Run on a clean checkout of `develop` at a known commit, before any P1–P6 +hot-path change lands (PRD #50: the baseline becomes unrecoverable the +moment optimization starts). + +1. **Allocate an exclusive node** on the partition your supervisor names as + production-representative — the partition is a runtime parameter, never + hard-coded into the script or this doc: + + ```sh + salloc --exclusive --partition= --time=00:30:00 + ``` + + `$SLURM_JOB_PARTITION` is then set for the rest of the allocation, and + `capture_baseline.py` picks it up automatically (or pass + `--slurm-partition ` explicitly). + +2. **Load the toolchain modules**: + + ```sh + module load foss-2023b googletest + ``` + + `foss-2023b` provides the compiler and the HDF5 the benchmark links + against; `googletest` is needed because the harness links + `al_contract_fixtures` (shared with the contract suite). + +3. **Configure and build in Release**, on GPFS (`/mnt/HPC_T2`), not on a + login-node home directory: + + ```sh + cd /mnt/HPC_T2//IMAS-Core + cmake --preset benchmarks + cmake --build --preset benchmarks + ``` + +4. **Capture**, recording the filesystem explicitly (auto-detection is + best-effort and GPFS mounts don't always self-report cleanly) and a + `machine-id` that names the cluster, not the specific login/compute node: + + ```sh + python3 benchmarks/capture_baseline.py \ + --binary build-benchmarks/benchmarks/al_benchmarks \ + --machine-id sdcc-gpfs \ + --filesystem gpfs + ``` + + No `--benchmark_min_time` here — let each scenario run its full + repetition count so the committed baseline reports a real distribution + (median, mean, stddev), per PRD #50 user story 8. + +5. **Verify before committing**: the script refuses (unless `--force`) to + overwrite a baseline already committed for this `machine-id` + commit SHA, + and warns to stderr if the working tree is dirty or the build isn't + Release. Resolve any such warning rather than force past it for a + measurement of record. + +6. **Commit** the resulting file under `benchmarks/baselines/sdcc-gpfs/` — + see [`baselines/README.md`](baselines/README.md) for the layout and how + later runs compare against it. + +## What gets recorded either way + +`captured_at`, `machine_id`, `os`, `cpu_model`, `compiler` (+ version), +`compiler_flags`, `build_type`, `hdf5_version`, `filesystem`, +`slurm_partition` (`null` off-cluster), `commit_sha`, `commit_dirty` — spliced +into the Google Benchmark JSON's `context` block as `imas_baseline_metadata`. +Compiler/flags/HDF5 version are read from the build directory's +`CMakeCache.txt` (found by walking up from `--binary`, or pass `--build-dir` +explicitly if your build layout doesn't nest the binary under it). diff --git a/benchmarks/baselines/README.md b/benchmarks/baselines/README.md new file mode 100644 index 00000000..ad17c994 --- /dev/null +++ b/benchmarks/baselines/README.md @@ -0,0 +1,66 @@ +# Committed baseline layout + +``` +benchmarks/baselines//.json +``` + +Each file is the raw Google Benchmark JSON that `al_benchmarks` emitted for +that run — Google Benchmark's own top-level `context` (date, host_name, +num_cpus, ...) and `benchmarks` (one entry per `BENCHMARK()` registration, +repetitions, and `_mean`/`_median`/`_stddev`/`_cv` rows) — with one field +spliced into `context`: + +```json +{ + "imas_baseline_metadata": { + "captured_at": "2026-07-20T09:50:42.347134+00:00", + "machine_id": "sdcc-gpfs", + "os": "Linux-5.14.0-x86_64", + "cpu_model": "Intel(R) Xeon(R) Gold 6248R", + "compiler": "GNU 12.3.0", + "compiler_flags": "-O3 -DNDEBUG", + "build_type": "Release", + "hdf5_version": "1.14.3", + "filesystem": "gpfs", + "slurm_partition": "iter", + "commit_sha": "7246824127a3339e6864f91238eb434a8ebb2afc", + "commit_dirty": false + } +} +``` + +(the object above nests under `context.imas_baseline_metadata`; only that +field is shown, not the full committed file) + +`imas_baseline_metadata` is what makes two files comparable — or tells you why +they aren't (different `compiler_flags`, different `filesystem`, etc). See +[`../BASELINE_CAPTURE.md`](../BASELINE_CAPTURE.md) for how these files are +produced. + +## Naming + +- `` is whatever `--machine-id` was passed to + `capture_baseline.py` (default: the sanitized hostname). Use a stable, + descriptive id for anything meant to be a measurement of record, e.g. + `sdcc-gpfs` for the ITER SDCC cluster, rather than a raw login-node + hostname that might change. +- `` is the full `git rev-parse HEAD` of the run. A dirty working + tree is still captured (for laptop iteration) but is flagged + `commit_dirty: true` in the metadata — that file is not reproducible from + the SHA alone and should not be treated as a measurement of record. + +## Comparing two baselines + +There is no comparison tool here by design (PRD #50: "Not tested: the +numeric results themselves... a manual/analysis step over the committed +JSON, not a CI gate"). To compare, match `benchmarks[].name` between two +files and look at the `_mean`/`_median` rows' `real_time`/`cpu_time`, e.g. +with `jq`: + +```sh +jq '.benchmarks[] | select(.name | endswith("_mean")) | {name, real_time, cpu_time}' \ + benchmarks/baselines/sdcc-gpfs/.json + +jq '.benchmarks[] | select(.name | endswith("_mean")) | {name, real_time, cpu_time}' \ + benchmarks/baselines/sdcc-gpfs/.json +``` diff --git a/benchmarks/capture_baseline.py b/benchmarks/capture_baseline.py new file mode 100755 index 00000000..ae80f754 --- /dev/null +++ b/benchmarks/capture_baseline.py @@ -0,0 +1,335 @@ +#!/usr/bin/env python3 +"""Run the al_benchmarks binary and capture a metadata-tagged baseline artifact. + +Wraps whatever Google Benchmark JSON the harness (benchmarks/al_benchmarks, +issues #52/#53) emits and tags it with the comparability metadata a committed +baseline needs (NORTH_STAR.md §8.4, issue #55): OS, CPU model, compiler + +flags, HDF5 version, filesystem, SLURM partition, and commit SHA. The tagged +JSON is written to the committed-artifact layout, keyed by machine identifier +and commit SHA: + + benchmarks/baselines//.json + +The SLURM partition is never hard-coded: pass --slurm-partition, or run under +an allocation that sets $SLURM_JOB_PARTITION, or leave both unset (recorded as +null -- e.g. a laptop smoke run has no partition at all). + +Comparing two baselines (dividing matching "real_time"/"cpu_time" entries) is +a manual analysis step over the committed JSON, not something this script +does (PRD #50 "Not tested: the numeric results themselves"). +""" + +from __future__ import annotations + +import argparse +import json +import os +import platform +import re +import socket +import subprocess +import sys +import tempfile +from datetime import datetime, timezone +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent +DEFAULT_OUTPUT_DIR = REPO_ROOT / "benchmarks" / "baselines" + + +def run_git(*args: str) -> str: + return subprocess.check_output( + ["git", *args], cwd=REPO_ROOT, text=True + ).strip() + + +def commit_metadata() -> dict: + sha = run_git("rev-parse", "HEAD") + dirty = bool(run_git("status", "--porcelain")) + return {"commit_sha": sha, "commit_dirty": dirty} + + +def sanitize_machine_id(raw: str) -> str: + slug = re.sub(r"[^a-zA-Z0-9]+", "-", raw).strip("-").lower() + return slug or "unknown-machine" + + +def best_effort_output(cmd: list[str]) -> str | None: + """Run `cmd`, returning stripped stdout, or None if it can't be run.""" + try: + return subprocess.check_output( + cmd, text=True, stderr=subprocess.STDOUT + ).strip() + except (OSError, subprocess.CalledProcessError): + return None + + +def detect_os() -> str: + return platform.platform() + + +def detect_cpu_model() -> str: + system = platform.system() + if system == "Darwin": + brand = best_effort_output(["sysctl", "-n", "machdep.cpu.brand_string"]) + if brand: + return brand + elif system == "Linux": + try: + with open("/proc/cpuinfo") as f: + for line in f: + if line.lower().startswith("model name"): + return line.split(":", 1)[1].strip() + except OSError: + pass + return platform.processor() or "unknown" + + +def read_cmake_cache(build_dir: Path) -> dict[str, str]: + cache_path = build_dir / "CMakeCache.txt" + entries = {} + if not cache_path.is_file(): + return entries + for line in cache_path.read_text().splitlines(): + line = line.strip() + if not line or line.startswith("#") or line.startswith("//"): + continue + if ":" not in line or "=" not in line: + continue + key_type, value = line.split("=", 1) + key = key_type.split(":", 1)[0] + entries[key] = value + return entries + + +def find_build_dir(binary: Path, explicit: Path | None) -> Path | None: + if explicit is not None: + return explicit + for candidate in binary.resolve().parents: + if (candidate / "CMakeCache.txt").is_file(): + return candidate + return None + + +def detect_compiler(cache: dict[str, str]) -> str: + compiler_id = cache.get("CMAKE_CXX_COMPILER_ID") + compiler_version = cache.get("CMAKE_CXX_COMPILER_VERSION") + if compiler_id and compiler_version: + return f"{compiler_id} {compiler_version}" + compiler_path = cache.get("CMAKE_CXX_COMPILER") + if compiler_path: + version_output = best_effort_output([compiler_path, "--version"]) + if version_output: + return version_output.splitlines()[0] + return "unknown" + + +def detect_compiler_flags(cache: dict[str, str]) -> str: + build_type = cache.get("CMAKE_BUILD_TYPE", "") + parts = [cache.get("CMAKE_CXX_FLAGS", "")] + if build_type: + parts.append(cache.get(f"CMAKE_CXX_FLAGS_{build_type.upper()}", "")) + return " ".join(p for p in parts if p).strip() or "unknown" + + +def detect_hdf5_version(cache: dict[str, str]) -> str: + hdf5_dir = cache.get("HDF5_DIR") + if hdf5_dir: + version_file = Path(hdf5_dir) / "hdf5-config-version.cmake" + if version_file.is_file(): + match = re.search( + r'set\(PACKAGE_VERSION\s+"([^"]+)"\)', version_file.read_text() + ) + if match: + return match.group(1) + version_output = best_effort_output(["h5dump", "--version"]) + if version_output: + match = re.search(r"Version\s+([0-9.]+)", version_output) + if match: + return match.group(1) + return "unknown" + + +def detect_filesystem(path: Path, override: str | None) -> str: + if override: + return override + if platform.system() == "Linux": + fstype = best_effort_output(["findmnt", "-no", "FSTYPE", "--target", str(path)]) + if fstype: + return fstype + elif platform.system() == "Darwin": + df_output = best_effort_output(["df", "-P", str(path)]) + mount_output = best_effort_output(["mount"]) + if df_output and mount_output: + device = df_output.splitlines()[-1].split()[0] + for line in mount_output.splitlines(): + if line.startswith(device + " "): + match = re.search(r"\(([^,]+)", line) + if match: + return match.group(1) + return "unknown" + + +def build_metadata(args: argparse.Namespace, build_dir: Path | None) -> dict[str, object]: + cache = read_cmake_cache(build_dir) if build_dir else {} + metadata = { + "captured_at": datetime.now(timezone.utc).isoformat(), + "machine_id": args.machine_id, + "os": detect_os(), + "cpu_model": detect_cpu_model(), + "compiler": detect_compiler(cache), + "compiler_flags": detect_compiler_flags(cache), + "build_type": cache.get("CMAKE_BUILD_TYPE", "unknown"), + "hdf5_version": detect_hdf5_version(cache), + "filesystem": detect_filesystem( + build_dir or REPO_ROOT, args.filesystem + ), + "slurm_partition": args.slurm_partition, + } + metadata.update(commit_metadata()) + return metadata + + +def run_benchmark(binary: Path, extra_args: list[str], out_path: Path) -> str | None: + """Run the benchmark binary; return an error message, or None on success.""" + try: + subprocess.run( + [ + str(binary), + f"--benchmark_out={out_path}", + "--benchmark_out_format=json", + *extra_args, + ], + check=True, + ) + except (OSError, subprocess.CalledProcessError) as exc: + return f"failed to run {binary}: {exc}" + return None + + +def merge_metadata(json_path: Path, metadata: dict[str, object]) -> dict: + result = json.loads(json_path.read_text()) + result.setdefault("context", {})["imas_baseline_metadata"] = metadata + return result + + +def parse_args(argv: list[str]) -> argparse.Namespace: + parser = argparse.ArgumentParser( + prog="capture_baseline.py", + description=( + "Run al_benchmarks and write a metadata-tagged, committable " + "baseline artifact under benchmarks/baselines/." + ), + ) + parser.add_argument( + "--binary", required=True, type=Path, help="path to the al_benchmarks executable" + ) + parser.add_argument( + "--build-dir", + type=Path, + default=None, + help="CMake build dir to read compiler/flags/HDF5 metadata from " + "(default: discovered by walking up from --binary for CMakeCache.txt)", + ) + parser.add_argument( + "--machine-id", + default=sanitize_machine_id(socket.gethostname()), + help="identifier this baseline is filed under (default: sanitized hostname)", + ) + parser.add_argument( + "--output-dir", + type=Path, + default=DEFAULT_OUTPUT_DIR, + help="root of the committed baseline layout (default: benchmarks/baselines)", + ) + parser.add_argument( + "--filesystem", + default=None, + help="filesystem name to record (default: best-effort auto-detect, " + "e.g. pass 'gpfs' explicitly on the cluster)", + ) + parser.add_argument( + "--slurm-partition", + default=None, + help="SLURM partition to record (default: $SLURM_JOB_PARTITION, or null " + "if unset -- this is always a runtime parameter, never hard-coded)", + ) + parser.add_argument( + "--force", + action="store_true", + help="overwrite an existing committed baseline file for this machine+commit", + ) + parser.add_argument( + "benchmark_args", + nargs=argparse.REMAINDER, + help="remaining args forwarded verbatim to al_benchmarks " + "(e.g. -- --benchmark_filter=Equilibrium --benchmark_min_time=1x)", + ) + args = parser.parse_args(argv) + if args.benchmark_args[:1] == ["--"]: + args.benchmark_args = args.benchmark_args[1:] + if args.slurm_partition is None: + args.slurm_partition = os.environ.get("SLURM_JOB_PARTITION") or None + return args + + +def main(argv: list[str]) -> int: + args = parse_args(argv) + + if not args.binary.is_file(): + print(f"error: benchmark binary not found: {args.binary}", file=sys.stderr) + return 1 + + build_dir = find_build_dir(args.binary, args.build_dir) + if build_dir is None: + print( + "warning: could not locate a CMakeCache.txt; compiler/flags/HDF5 " + "metadata will be recorded as 'unknown' (pass --build-dir to fix this)", + file=sys.stderr, + ) + + metadata = build_metadata(args, build_dir) + + if metadata["commit_dirty"]: + print( + "warning: working tree is dirty -- this baseline is not reproducible " + "from the recorded commit SHA alone (metadata still records commit_dirty=true)", + file=sys.stderr, + ) + if metadata["build_type"] != "Release": + print( + f"warning: build type is '{metadata['build_type']}', not Release -- " + "NORTH_STAR.md §8.4 numbers are only comparable across Release builds", + file=sys.stderr, + ) + + machine_dir = args.output_dir / args.machine_id + out_path = machine_dir / f"{metadata['commit_sha']}.json" + if out_path.exists() and not args.force: + print( + f"error: {out_path} already exists (pass --force to overwrite a " + "committed baseline)", + file=sys.stderr, + ) + return 1 + + with tempfile.TemporaryDirectory() as tmp: + raw_json = Path(tmp) / "benchmark_raw.json" + error = run_benchmark(args.binary, args.benchmark_args, raw_json) + if error is not None: + print(f"error: {error}", file=sys.stderr) + return 1 + tagged = merge_metadata(raw_json, metadata) + + machine_dir.mkdir(parents=True, exist_ok=True) + out_path.write_text(json.dumps(tagged, indent=2) + "\n") + + print(f"wrote baseline: {out_path}") + width = max(len(key) for key in metadata) + for key, value in metadata.items(): + print(f" {key:<{width}} = {value}") + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) From d5e53d127ab9bedb1a15d5293e21207ef348ffa9 Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Mon, 20 Jul 2026 13:38:51 +0200 Subject: [PATCH 07/12] Fix benchmark baseline capture metadata (cherry picked from commit c0ee66c80725a6fc44b37c333789e8e80903a549) --- benchmarks/BASELINE_CAPTURE.md | 9 ++- benchmarks/CMakeLists.txt | 5 ++ benchmarks/baselines/README.md | 5 +- benchmarks/capture_baseline.py | 15 ++++- benchmarks/test_capture_baseline.py | 99 +++++++++++++++++++++++++++++ 5 files changed, 128 insertions(+), 5 deletions(-) create mode 100644 benchmarks/test_capture_baseline.py diff --git a/benchmarks/BASELINE_CAPTURE.md b/benchmarks/BASELINE_CAPTURE.md index 4b93a896..138a7fc4 100644 --- a/benchmarks/BASELINE_CAPTURE.md +++ b/benchmarks/BASELINE_CAPTURE.md @@ -34,6 +34,11 @@ filesystem, commit SHA/dirty flag, `slurm_partition: null`) and the path it wrote to. On a laptop `commit_dirty` will often be `true` — that's expected and is exactly why this file is a smoke artifact, not a baseline of record. +On macOS, the script detects the Apple chip through `system_profiler` when the +usual CPU-brand `sysctl` is unavailable. If automatic detection is blocked in +your environment, pass the precise model explicitly, for example +`--cpu-model "Apple M5 Pro"`. + ## ITER SDCC cluster (measurement of record) Run on a clean checkout of `develop` at a known commit, before any P1–P6 @@ -104,4 +109,6 @@ moment optimization starts). into the Google Benchmark JSON's `context` block as `imas_baseline_metadata`. Compiler/flags/HDF5 version are read from the build directory's `CMakeCache.txt` (found by walking up from `--binary`, or pass `--build-dir` -explicitly if your build layout doesn't nest the binary under it). +explicitly if your build layout doesn't nest the binary under it). Machine IDs +are normalized to lowercase letters, digits, and dashes, ensuring every +artifact remains inside the documented baseline layout. diff --git a/benchmarks/CMakeLists.txt b/benchmarks/CMakeLists.txt index 4d7cb55f..605a9430 100644 --- a/benchmarks/CMakeLists.txt +++ b/benchmarks/CMakeLists.txt @@ -22,6 +22,8 @@ set(BENCHMARK_ENABLE_GTEST_TESTS OFF CACHE BOOL "" FORCE) set(BENCHMARK_ENABLE_WERROR OFF CACHE BOOL "" FORCE) FetchContent_MakeAvailable(googlebenchmark) +find_package(Python3 COMPONENTS Interpreter REQUIRED) + ######################################################################################################################## # The benchmark executable. # @@ -78,3 +80,6 @@ add_test( NAME benchmark_smoke_validate COMMAND ${CMAKE_COMMAND} -DINPUT=${_smoke_json} -P ${CMAKE_CURRENT_SOURCE_DIR}/validate_benchmark_json.cmake ) set_tests_properties( benchmark_smoke_validate PROPERTIES FIXTURES_REQUIRED BENCHMARK_SMOKE_JSON ) + +add_test( NAME baseline_capture_unit + COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/test_capture_baseline.py ) diff --git a/benchmarks/baselines/README.md b/benchmarks/baselines/README.md index ad17c994..f9bd3241 100644 --- a/benchmarks/baselines/README.md +++ b/benchmarks/baselines/README.md @@ -39,8 +39,9 @@ produced. ## Naming -- `` is whatever `--machine-id` was passed to - `capture_baseline.py` (default: the sanitized hostname). Use a stable, +- `` is the normalized value of `--machine-id` (default: the + sanitized hostname). It contains only lowercase letters, digits, and dashes. + Use a stable, descriptive id for anything meant to be a measurement of record, e.g. `sdcc-gpfs` for the ITER SDCC cluster, rather than a raw login-node hostname that might change. diff --git a/benchmarks/capture_baseline.py b/benchmarks/capture_baseline.py index ae80f754..69d7b453 100755 --- a/benchmarks/capture_baseline.py +++ b/benchmarks/capture_baseline.py @@ -74,6 +74,11 @@ def detect_cpu_model() -> str: brand = best_effort_output(["sysctl", "-n", "machdep.cpu.brand_string"]) if brand: return brand + hardware = best_effort_output(["system_profiler", "SPHardwareDataType"]) + if hardware: + chip = re.search(r"^\s*Chip:\s*(.+)$", hardware, re.MULTILINE) + if chip: + return chip.group(1).strip() elif system == "Linux": try: with open("/proc/cpuinfo") as f: @@ -176,7 +181,7 @@ def build_metadata(args: argparse.Namespace, build_dir: Path | None) -> dict[str "captured_at": datetime.now(timezone.utc).isoformat(), "machine_id": args.machine_id, "os": detect_os(), - "cpu_model": detect_cpu_model(), + "cpu_model": args.cpu_model or detect_cpu_model(), "compiler": detect_compiler(cache), "compiler_flags": detect_compiler_flags(cache), "build_type": cache.get("CMAKE_BUILD_TYPE", "unknown"), @@ -234,7 +239,13 @@ def parse_args(argv: list[str]) -> argparse.Namespace: parser.add_argument( "--machine-id", default=sanitize_machine_id(socket.gethostname()), - help="identifier this baseline is filed under (default: sanitized hostname)", + type=sanitize_machine_id, + help="identifier this baseline is filed under (always sanitized; default: hostname)", + ) + parser.add_argument( + "--cpu-model", + default=None, + help="CPU model to record when automatic detection is unavailable", ) parser.add_argument( "--output-dir", diff --git a/benchmarks/test_capture_baseline.py b/benchmarks/test_capture_baseline.py new file mode 100644 index 00000000..9478e407 --- /dev/null +++ b/benchmarks/test_capture_baseline.py @@ -0,0 +1,99 @@ +#!/usr/bin/env python3 +"""Hermetic tests for the baseline-capture command-line workflow.""" + +from __future__ import annotations + +import importlib.util +import json +import tempfile +import unittest +from pathlib import Path +from unittest import mock + + +SCRIPT = Path(__file__).with_name("capture_baseline.py") +SPEC = importlib.util.spec_from_file_location("capture_baseline", SCRIPT) +assert SPEC is not None and SPEC.loader is not None +capture_baseline = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(capture_baseline) + + +class CaptureBaselineTest(unittest.TestCase): + def setUp(self) -> None: + self.temp_dir = tempfile.TemporaryDirectory() + self.root = Path(self.temp_dir.name) + self.binary = self.root / "fake_benchmark.py" + self.binary.write_text( + "#!/usr/bin/env python3\n" + "import json\n" + "import sys\n" + "from pathlib import Path\n" + "out = next(arg.split('=', 1)[1] for arg in sys.argv " + "if arg.startswith('--benchmark_out='))\n" + "Path(out).write_text(json.dumps({'context': {'source': 'fake'}, " + "'benchmarks': []}))\n" + ) + self.binary.chmod(0o755) + + def tearDown(self) -> None: + self.temp_dir.cleanup() + + def test_capture_tags_json_and_protects_machine_commit_artifact(self) -> None: + output_dir = self.root / "baselines" + metadata = { + "captured_at": "2026-07-20T10:00:00+00:00", + "machine_id": "machine-a", + "os": "test-os", + "cpu_model": "test-cpu", + "compiler": "test-compiler", + "compiler_flags": "-O3", + "build_type": "Release", + "hdf5_version": "1.14.3", + "filesystem": "testfs", + "slurm_partition": None, + "commit_sha": "deadbeef", + "commit_dirty": False, + } + common_args = [ + "--binary", str(self.binary), + "--machine-id", "../Machine A", + "--output-dir", str(output_dir), + ] + args = [ + *common_args, + "--", "--benchmark_min_time=1x", + ] + + with mock.patch.object(capture_baseline, "build_metadata", return_value=metadata): + self.assertEqual(capture_baseline.main(args), 0) + + artifact = output_dir / "machine-a" / "deadbeef.json" + self.assertTrue(artifact.is_file()) + captured = json.loads(artifact.read_text()) + self.assertEqual(captured["context"]["source"], "fake") + self.assertEqual(captured["context"]["imas_baseline_metadata"], metadata) + + self.assertEqual(capture_baseline.main(args), 1) + self.assertEqual( + capture_baseline.main([*common_args, "--force", "--", "--benchmark_min_time=1x"]), + 0, + ) + + def test_detect_cpu_model_reads_apple_silicon_chip(self) -> None: + with mock.patch.object(capture_baseline.platform, "system", return_value="Darwin"), \ + mock.patch.object( + capture_baseline, + "best_effort_output", + side_effect=[None, "Hardware:\n Chip: Apple M5 Pro\n"], + ): + self.assertEqual(capture_baseline.detect_cpu_model(), "Apple M5 Pro") + + def test_explicit_machine_id_is_sanitized(self) -> None: + args = capture_baseline.parse_args( + ["--binary", str(self.binary), "--machine-id", "../../outside"] + ) + self.assertEqual(args.machine_id, "outside") + + +if __name__ == "__main__": + unittest.main() From b04948bfbbecc1a47db2fdd889297ed08e986969 Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Tue, 21 Jul 2026 10:52:21 +0200 Subject: [PATCH 08/12] Extract shared per-backend benchmark boilerplate into bench_common.h Moves BenchBackend, the HDF5/Memory URI builder, and pulse-open lifecycle setup out of bench_equilibrium_read.cpp into bench_common.h so the three follow-on benchmarks (issue #57) can reuse them instead of re-deriving the same boilerplate. No change to the existing whole-IDS-read benchmark's behavior or reported numbers (issue #58). (cherry picked from commit 9554df21bdce47449763d2c3f5e9260b90a9d470) --- benchmarks/bench_common.h | 74 +++++++++++++++++++++++++-- benchmarks/bench_equilibrium_read.cpp | 55 +++----------------- 2 files changed, 76 insertions(+), 53 deletions(-) diff --git a/benchmarks/bench_common.h b/benchmarks/bench_common.h index 7b248a3a..3dab4c0d 100644 --- a/benchmarks/bench_common.h +++ b/benchmarks/bench_common.h @@ -1,12 +1,21 @@ -// Shared by every bench_*.cpp TU in al_benchmarks: the one status-checking -// helper each benchmark uses to turn a failing al_status_t into a thrown -// exception (Google Benchmark reports the failure instead of silently timing -// garbage). Header-only so it carries no extra link surface. +// Shared by every bench_*.cpp TU in al_benchmarks: the status-checking helper +// each benchmark uses to turn a failing al_status_t into a thrown exception +// (Google Benchmark reports the failure instead of silently timing garbage), +// plus the per-backend (HDF5 vs. Memory) URI construction and pulse-open +// boilerplate that every whole-IDS-scoped benchmark needs identically (issue +// #58, prefactor for PRD #57's write/single-slice/AOS-traversal benchmarks). +// Header-only so it carries no extra link surface. #ifndef AL_BENCHMARKS_COMMON_H #define AL_BENCHMARKS_COMMON_H +#include "al_contract_legacy_storage.h" + +#include #include +#include +#include +#include #include #include @@ -20,6 +29,63 @@ inline void CheckOk(const al_status_t& status, const char* what) { } } +enum class BenchBackend { kHdf5, kMemory }; + +// HDF5 resolves FORCE_CREATE_PULSE against a real on-disk legacy tree, so it +// needs `base` alive for the URI to mean anything. Memory never touches disk +// (bench_write_read.cpp's MemoryUri established the pattern): an arbitrary +// absolute-looking `user` sidesteps the legacy URI builder's getpwnam() +// fallback for an empty/unknown user, without paying for a real temp dir. +inline std::string BuildUri(BenchBackend backend, const al_contract::PulseId& pulse, + const al_contract::LegacyPulseDirectory* base) { + char* uri = nullptr; + if (backend == BenchBackend::kHdf5) { + assert(base != nullptr && "HDF5 backend requires a legacy pulse directory for its URI"); + CheckOk(al_build_uri_from_legacy_parameters( + HDF5_BACKEND, pulse.pulse, pulse.run, base->str().c_str(), + pulse.database.c_str(), pulse.version.c_str(), + /*options=*/"", &uri), + "al_build_uri_from_legacy_parameters"); + } else { + CheckOk(al_build_uri_from_legacy_parameters( + MEMORY_BACKEND, pulse.pulse, pulse.run, "/al-benchmarks", + pulse.database.c_str(), pulse.version.c_str(), + /*options=*/"", &uri), + "al_build_uri_from_legacy_parameters"); + } + std::string result(uri ? uri : ""); + free(uri); + return result; +} + +// An open pulse context plus (HDF5 only) the temp legacy directory backing +// it. `base` is a unique_ptr rather than the more obvious optional<> so +// BenchPulse stays movable out of OpenBenchPulse regardless of +// LegacyPulseDirectory's own non-movability (its destructor removes the temp +// dir on every exit path, so it must outlive the pulse being open). +struct BenchPulse { + int pulse_ctx = -1; + std::unique_ptr base; +}; + +// Per-backend pulse lifecycle setup shared by every whole-IDS-scoped +// benchmark: materialize the on-disk legacy tree (HDF5 only), build the +// backend's URI, and open the pulse with FORCE_CREATE_PULSE. Callers write +// their fixture and run their timed region on the returned pulse_ctx, then +// close it themselves (close is operation-specific: some benchmarks time +// around it, none should hide it in a header). +inline BenchPulse OpenBenchPulse(BenchBackend backend, const al_contract::PulseId& pulse) { + BenchPulse result; + if (backend == BenchBackend::kHdf5) { + result.base = std::make_unique(); + result.base->make_legacy_tree(pulse); + } + const std::string uri = BuildUri(backend, pulse, result.base.get()); + CheckOk(al_begin_dataentry_action(uri.c_str(), FORCE_CREATE_PULSE, &result.pulse_ctx), + "al_begin_dataentry_action"); + return result; +} + } // namespace al_bench #endif // AL_BENCHMARKS_COMMON_H diff --git a/benchmarks/bench_equilibrium_read.cpp b/benchmarks/bench_equilibrium_read.cpp index b7a75a65..923d682c 100644 --- a/benchmarks/bench_equilibrium_read.cpp +++ b/benchmarks/bench_equilibrium_read.cpp @@ -26,53 +26,12 @@ #include -#include -#include -#include - -namespace { - +using al_bench::BenchBackend; using al_bench::CheckOk; -enum class BenchBackend { kHdf5, kMemory }; - -// HDF5 resolves FORCE_CREATE_PULSE against a real on-disk legacy tree, so it -// needs `base` alive for the URI to mean anything. Memory never touches disk -// (bench_write_read.cpp's MemoryUri established the pattern): an arbitrary -// absolute-looking `user` sidesteps the legacy URI builder's getpwnam() -// fallback for an empty/unknown user, without paying for a real temp dir. -std::string BuildUri(BenchBackend backend, const al_contract::PulseId& pulse, - const al_contract::LegacyPulseDirectory* base) { - char* uri = nullptr; - if (backend == BenchBackend::kHdf5) { - CheckOk(al_build_uri_from_legacy_parameters( - HDF5_BACKEND, pulse.pulse, pulse.run, base->str().c_str(), - pulse.database.c_str(), pulse.version.c_str(), - /*options=*/"", &uri), - "al_build_uri_from_legacy_parameters"); - } else { - CheckOk(al_build_uri_from_legacy_parameters( - MEMORY_BACKEND, pulse.pulse, pulse.run, "/al-benchmarks", - pulse.database.c_str(), pulse.version.c_str(), - /*options=*/"", &uri), - "al_build_uri_from_legacy_parameters"); - } - std::string result(uri ? uri : ""); - free(uri); - return result; -} - -} // namespace - void BM_EquilibriumWholeIdsRead(benchmark::State& state, BenchBackend backend) { const al_contract::PulseId pulse{/*database=*/"bench", /*version=*/"3", /*pulse=*/1, /*run=*/0}; - std::optional base; - if (backend == BenchBackend::kHdf5) { - base.emplace(); - base->make_legacy_tree(pulse); - } - const std::string uri = BuildUri(backend, pulse, base ? &*base : nullptr); // --- untimed setup: open once, write the reference equilibrium once ------ // Memory keeps the fixed pulse identity across repetitions unlike @@ -80,22 +39,20 @@ void BM_EquilibriumWholeIdsRead(benchmark::State& state, BenchBackend backend) { // no-op (src/memory_backend.h), so correctness across the 20 Repetitions // rests on FORCE_CREATE_PULSE clearing the reused entry's idsMap on reopen, // not on pulse identities never colliding. - int pulse_ctx = -1; - CheckOk(al_begin_dataentry_action(uri.c_str(), FORCE_CREATE_PULSE, &pulse_ctx), - "al_begin_dataentry_action"); - CheckOk(equilibrium_seed::write(pulse_ctx), "equilibrium_seed::write"); + al_bench::BenchPulse bp = al_bench::OpenBenchPulse(backend, pulse); + CheckOk(equilibrium_seed::write(bp.pulse_ctx), "equilibrium_seed::write"); // --- timed region: begin_global_action(READ) -> every leaf -> end_action - std::vector records; for (auto _ : state) { - CheckOk(equilibrium_seed::read_back(pulse_ctx, &records), + CheckOk(equilibrium_seed::read_back(bp.pulse_ctx, &records), "equilibrium_seed::read_back"); benchmark::DoNotOptimize(records); } // --- untimed teardown ------------------------------------------------------ - CheckOk(al_close_pulse(pulse_ctx, CLOSE_PULSE), "al_close_pulse"); - // base's destructor (HDF5 only) removes the temp directory on every exit path. + CheckOk(al_close_pulse(bp.pulse_ctx, CLOSE_PULSE), "al_close_pulse"); + // bp.base's destructor (HDF5 only) removes the temp directory on every exit path. } // Repetitions(N) makes Google Benchmark report the distribution (median, // mean, stddev, iteration count) across N independent setup/read/teardown From 206ea998bee2f06cbdca2a94e9fc143549af7319 Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Tue, 21 Jul 2026 11:14:27 +0200 Subject: [PATCH 09/12] Add single time-slice-element read benchmark (HDF5 + Memory) (issue #60) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reads one time_slice element's full leaf set (timebase leaf, radial profile, scalar quantity, constraints subtree) by index, node-by-node, through the public C ABI -- the shared "before" baseline both P2's al_read_subtree and P3's slice-mode buffering (NORTH_STAR.md §7) will diff against, since neither optimization exists yet and this node-by-node walk is the only legal way to fetch one AOS element today. (cherry picked from commit f15d86af8fb8aa5cbded114f3597c30322dc5010) --- benchmarks/CMakeLists.txt | 6 ++ benchmarks/bench_timeslice_element_read.cpp | 102 ++++++++++++++++++++ 2 files changed, 108 insertions(+) create mode 100644 benchmarks/bench_timeslice_element_read.cpp diff --git a/benchmarks/CMakeLists.txt b/benchmarks/CMakeLists.txt index 605a9430..78762ca9 100644 --- a/benchmarks/CMakeLists.txt +++ b/benchmarks/CMakeLists.txt @@ -37,12 +37,18 @@ find_package(Python3 COMPONENTS Interpreter REQUIRED) # operation against both HDF5 (needs the filesystem -- std::filesystem temp # dirs) and, as a zero-I/O control (issue #54), the in-RAM Memory backend. # +# bench_timeslice_element_read.cpp (issue #60) is the shared "before" +# baseline for the future P2 (al_read_subtree) and P3 (slice-mode buffering) +# optimizations (NORTH_STAR.md §7): one time_slice element's full leaf set, +# read node-by-node by index, against both HDF5 and Memory. +# # bench_main.cpp holds the one BENCHMARK_MAIN() shared by every bench_*.cpp # TU in this executable. add_executable( al_benchmarks bench_write_read.cpp bench_equilibrium_read.cpp + bench_timeslice_element_read.cpp bench_main.cpp ) target_link_libraries( al_benchmarks PRIVATE al_contract_fixtures benchmark::benchmark ) add_dependencies( al_benchmarks al ) diff --git a/benchmarks/bench_timeslice_element_read.cpp b/benchmarks/bench_timeslice_element_read.cpp new file mode 100644 index 00000000..90c5cf80 --- /dev/null +++ b/benchmarks/bench_timeslice_element_read.cpp @@ -0,0 +1,102 @@ +// The shared "before" baseline for two future optimizations (issue #60, +// PRD #57): a single time_slice element's full leaf set -- its own timebase +// leaf, radial profile array, scalar quantity, and constraints subtree -- +// read by index, node-by-node, through the public C ABI. Today this +// node-by-node walk is the *only* legal way to fetch one AOS element: no +// al_read_subtree call exists yet (NORTH_STAR.md §7 P2) and HDF5 slice-mode +// buffering hasn't been built yet either (§7 P3). Both future optimizations +// diff against BM_TimeSliceElementRead below rather than against two +// near-identical registrations of their own. +// +// Reuses equilibrium_seed.h verbatim (issue #51) for the fixture, exactly as +// bench_equilibrium_read.cpp does, so the workload never drifts from what the +// contract suite already pins. Only the read shape differs: where +// BM_EquilibriumWholeIdsRead walks every time_slice element, +// BM_TimeSliceElementRead selects one (the last, kNSlices - 1, so the +// benchmark exercises al_iterate_over_arraystruct actually moving the +// cursor rather than reading the trivial post-begin default position) and +// reads only that element's five leaves. +// +// Timed region = one operation (issue #53, PRD #50 user story 6): the +// reference equilibrium is written and the data entry opened once in untimed +// setup; only begin_global_action(READ) -> begin_arraystruct_action -> +// iterate to the target index -> read every leaf of that element -> +// end_action(aos) -> end_action(op) is inside the `for (auto _ : state)` loop. +// +// Backend is a sub-axis of the operation (PRD #50 "structure: +// scenario-primary, backend as a sub-axis"): BM_TimeSliceElementRead is the +// one operation body, HDF5 and Memory are two BENCHMARK_CAPTURE +// registrations of it differing only in which backend id the URI is built +// for. + +#include "bench_common.h" +#include "al_contract_abi.h" +#include "al_contract_legacy_storage.h" +#include "equilibrium_seed.h" + +#include +#include + +#include + +using al_bench::BenchBackend; +using al_bench::CheckOk; + +void BM_TimeSliceElementRead(benchmark::State& state, BenchBackend backend) { + const al_contract::PulseId pulse{/*database=*/"bench", /*version=*/"3", + /*pulse=*/1, /*run=*/0}; + constexpr int kTargetSlice = equilibrium_seed::kNSlices - 1; + + // --- untimed setup: open once, write the reference equilibrium once ------ + al_bench::BenchPulse bp = al_bench::OpenBenchPulse(backend, pulse); + CheckOk(equilibrium_seed::write(bp.pulse_ctx), "equilibrium_seed::write"); + + // --- timed region: begin_global_action(READ) -> select element -> + // read every leaf of that element -> end_action ---------------------- + std::vector shape; + std::vector data; + for (auto _ : state) { + int op = -1; + CheckOk(al_begin_global_action(bp.pulse_ctx, equilibrium_seed::kIds, "", + READ_OP, &op), + "al_begin_global_action"); + + int size = 0; + int aos_ctx = -1; + CheckOk(al_begin_arraystruct_action(op, equilibrium_seed::kAos, + equilibrium_seed::kAosTime, &size, + &aos_ctx), + "al_begin_arraystruct_action"); + // `step` is relative to the cursor's current position, not an absolute + // index (al_lowlevel.h: "iteration step size, typically=1"); this only + // lands on kTargetSlice because al_begin_arraystruct_action always opens + // at index 0. + CheckOk(al_iterate_over_arraystruct(aos_ctx, kTargetSlice), + "al_iterate_over_arraystruct"); + + auto read_leaf = [&](const char* path, int rank, const char* what) { + CheckOk(al_contract::read_data(aos_ctx, path, rank, &shape, &data), + what); + benchmark::DoNotOptimize(data); + }; + read_leaf(equilibrium_seed::kSliceTime, 0, "read time"); + read_leaf(equilibrium_seed::kPsi, 1, "read profiles_1d/psi"); + read_leaf(equilibrium_seed::kIp, 0, "read global_quantities/ip"); + read_leaf(equilibrium_seed::kMeasured, 0, "read constraints/ip/measured"); + read_leaf(equilibrium_seed::kWeight, 0, "read constraints/ip/weight"); + + CheckOk(al_end_action(aos_ctx), "al_end_action(aos_ctx)"); + CheckOk(al_end_action(op), "al_end_action(op)"); + } + + // --- untimed teardown ------------------------------------------------------ + CheckOk(al_close_pulse(bp.pulse_ctx, CLOSE_PULSE), "al_close_pulse"); + // bp.base's destructor (HDF5 only) removes the temp directory on every exit path. +} +// Repetitions(N) makes Google Benchmark report the distribution (median, +// mean, stddev, iteration count) across N independent read cycles, not a +// bare single-shot mean (issue #53, PRD #50 user story 8). +BENCHMARK_CAPTURE(BM_TimeSliceElementRead, HDF5, BenchBackend::kHdf5) + ->Repetitions(20); +BENCHMARK_CAPTURE(BM_TimeSliceElementRead, Memory, BenchBackend::kMemory) + ->Repetitions(20); From 984bc81a153a07a606d2114093d11b467914a56e Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Tue, 21 Jul 2026 15:12:33 +0200 Subject: [PATCH 10/12] Add whole-IDS write benchmark (HDF5 + Memory) (issue #59) Write-side counterpart to bench_equilibrium_read.cpp: pulse opened once (untimed), equilibrium_seed::write repeated inside the timed loop on the same open context, closed once (untimed), mirroring the read benchmark's discipline exactly. An explicit extra untimed write after the loop guarantees at least two writes to the same context on every run -- including the CTest smoke run, which caps the timed loop at exactly one iteration -- so the post-loop structural-hash check against equilibrium_seed::expected_hash() is a real, non-vacuous verification that repeated writes never accumulate stale data, on both backends. Surfaced (and relies on the fix for) a genuine HDF5Writer bug where a second whole-IDS write to an already-populated open pulse always re-attempted H5Dcreate2 instead of opening the existing dataset -- fixed separately as part of issue #60's concurrent work (commit 353a66d). (cherry picked from commit e7cfb7afc4eb9250f40324b6b88c25f97bcdc7b2) --- benchmarks/CMakeLists.txt | 6 ++ benchmarks/bench_equilibrium_write.cpp | 85 ++++++++++++++++++++++++++ 2 files changed, 91 insertions(+) create mode 100644 benchmarks/bench_equilibrium_write.cpp diff --git a/benchmarks/CMakeLists.txt b/benchmarks/CMakeLists.txt index 78762ca9..1a44d27f 100644 --- a/benchmarks/CMakeLists.txt +++ b/benchmarks/CMakeLists.txt @@ -37,6 +37,11 @@ find_package(Python3 COMPONENTS Interpreter REQUIRED) # operation against both HDF5 (needs the filesystem -- std::filesystem temp # dirs) and, as a zero-I/O control (issue #54), the in-RAM Memory backend. # +# bench_equilibrium_write.cpp is the write-side counterpart (issue #59): the +# same equilibrium_seed.h fixture, written repeatedly instead of read, +# against both backends, sharing bench_equilibrium_read.cpp's per-backend +# boilerplate via bench_common.h (issue #58). +# # bench_timeslice_element_read.cpp (issue #60) is the shared "before" # baseline for the future P2 (al_read_subtree) and P3 (slice-mode buffering) # optimizations (NORTH_STAR.md §7): one time_slice element's full leaf set, @@ -48,6 +53,7 @@ find_package(Python3 COMPONENTS Interpreter REQUIRED) add_executable( al_benchmarks bench_write_read.cpp bench_equilibrium_read.cpp + bench_equilibrium_write.cpp bench_timeslice_element_read.cpp bench_main.cpp ) target_link_libraries( al_benchmarks PRIVATE al_contract_fixtures benchmark::benchmark ) diff --git a/benchmarks/bench_equilibrium_write.cpp b/benchmarks/bench_equilibrium_write.cpp new file mode 100644 index 00000000..2e9b405c --- /dev/null +++ b/benchmarks/bench_equilibrium_write.cpp @@ -0,0 +1,85 @@ +// The write-side counterpart to bench_equilibrium_read.cpp (issue #59, PRD +// #57 user stories 2-5): whole-IDS write of the reference equilibrium +// fixture, driven strictly at the public C ABI (al_lowlevel.h), reusing +// equilibrium_seed.h verbatim and bench_common.h's shared per-backend +// pulse-open helper (issue #58). +// +// Timed region mirrors the read benchmark's discipline exactly: the pulse is +// opened once (FORCE_CREATE_PULSE) and closed once, both untimed; only the +// repeated equilibrium_seed::write call sits inside the +// `for (auto _ : state)` loop, so open/close cost never contaminates the +// write measurement. +// +// Repeated writes to the same open pulse context must never accumulate stale +// data from a prior write (PRD #57 user story 5): ALData::writeData clears +// its buffer before repopulating (src/memory_backend.cpp) and the +// array-of-structures path replaces rather than appends elements, for any +// whole-IDS (non-slice) write. This is checked once, untimed, after the timed +// loop -- but the timed loop's own iteration count isn't a reliable way to +// exercise "repeated": the CTest smoke entry caps every benchmark at exactly +// one iteration (--benchmark_min_time=1x), which would make the check +// vacuous (one write trivially matches one write's hash) on the one run this +// repo executes automatically. So an explicit second untimed write happens +// after the loop, guaranteeing at least two writes to the same context +// regardless of how many timed iterations ran, before a read-back of the +// resulting structural hash is compared against equilibrium_seed's +// expected_hash() exactly, on both backends. The hash folds in AOS size +// (equilibrium_seed.h), so an accumulation bug (e.g. elements appended +// instead of replaced) would drift it and this check -- which the existing +// CTest smoke entry already runs -- would throw. + +#include "bench_common.h" +#include "equilibrium_seed.h" + +#include +#include + +#include + +#include +#include + +using al_bench::BenchBackend; +using al_bench::CheckOk; + +void BM_EquilibriumWholeIdsWrite(benchmark::State& state, BenchBackend backend) { + // pulse=2 (distinct from bench_equilibrium_read.cpp's pulse=1) so the two + // benchmarks' Memory-backend entries never share an identity within the + // same al_benchmarks process. + const al_contract::PulseId pulse{/*database=*/"bench", /*version=*/"3", + /*pulse=*/2, /*run=*/0}; + + // --- untimed setup: open once ---------------------------------------------- + al_bench::BenchPulse bp = al_bench::OpenBenchPulse(backend, pulse); + + // --- timed region: repeated whole-IDS write on the same open context ----- + for (auto _ : state) { + CheckOk(equilibrium_seed::write(bp.pulse_ctx), "equilibrium_seed::write"); + } + + // --- untimed correctness check: no stale data survives repeated writes --- + // Force a second write regardless of the timed loop's iteration count (see + // top-of-file comment), then verify no staleness accumulated. + CheckOk(equilibrium_seed::write(bp.pulse_ctx), "equilibrium_seed::write"); + uint64_t hash = 0; + CheckOk(equilibrium_seed::read_and_hash(bp.pulse_ctx, &hash), + "equilibrium_seed::read_and_hash"); + if (hash != equilibrium_seed::expected_hash()) { + throw std::runtime_error( + "BM_EquilibriumWholeIdsWrite: repeated-write structural hash does " + "not match expected_hash() -- repeated writes may have accumulated " + "stale data"); + } + + // --- untimed teardown ------------------------------------------------------ + CheckOk(al_close_pulse(bp.pulse_ctx, CLOSE_PULSE), "al_close_pulse"); + // bp.base's destructor (HDF5 only) removes the temp directory on every exit path. +} +// Repetitions(N) makes Google Benchmark report the distribution (median, +// mean, stddev, iteration count) across N independent setup/write/teardown +// cycles, not a bare single-shot mean (issue #59, PRD #57 user story 10), +// mirroring bench_equilibrium_read.cpp. +BENCHMARK_CAPTURE(BM_EquilibriumWholeIdsWrite, HDF5, BenchBackend::kHdf5) + ->Repetitions(20); +BENCHMARK_CAPTURE(BM_EquilibriumWholeIdsWrite, Memory, BenchBackend::kMemory) + ->Repetitions(20); From 4a75a637c404aae5317c2667996b1893a4df6055 Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Tue, 21 Jul 2026 15:21:55 +0200 Subject: [PATCH 11/12] Add AOS-heavy traversal benchmark (HDF5 + Memory) (issue #61) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Iterates every time_slice element via al_iterate_over_arraystruct, reading all five leaves of each, against both backends -- the "before" baseline for P1/P4's round-trip-count reduction (NORTH_STAR.md §7) and the macro signal that would justify a future P5 per-call primitive micro-benchmark if it comes back slow. Functionally the same traversal bench_equilibrium_read.cpp already drives via equilibrium_seed::read_back, isolated and named as its own operation because its coverage target (P1/P4 round-trip count) is distinct from "read everything in the IDS" as a whole. Reuses equilibrium_seed.h and bench_common.h's shared per-backend boilerplate (issue #58) unchanged; added as a source to the existing al_benchmarks executable, so the existing CTest smoke check and baseline-capture script cover it automatically with no changes to either. (cherry picked from commit 2d7e07cf4595ff6673e2a7efd0638eb01c685627) --- benchmarks/CMakeLists.txt | 6 ++ benchmarks/bench_aos_traversal.cpp | 107 +++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+) create mode 100644 benchmarks/bench_aos_traversal.cpp diff --git a/benchmarks/CMakeLists.txt b/benchmarks/CMakeLists.txt index 1a44d27f..403c6708 100644 --- a/benchmarks/CMakeLists.txt +++ b/benchmarks/CMakeLists.txt @@ -47,6 +47,11 @@ find_package(Python3 COMPONENTS Interpreter REQUIRED) # optimizations (NORTH_STAR.md §7): one time_slice element's full leaf set, # read node-by-node by index, against both HDF5 and Memory. # +# bench_aos_traversal.cpp (issue #61) is the "before" baseline for P1/P4's +# round-trip-count reduction (NORTH_STAR.md §7): every time_slice element, +# every field, visited via al_iterate_over_arraystruct, against both HDF5 and +# Memory. +# # bench_main.cpp holds the one BENCHMARK_MAIN() shared by every bench_*.cpp # TU in this executable. @@ -55,6 +60,7 @@ add_executable( al_benchmarks bench_equilibrium_read.cpp bench_equilibrium_write.cpp bench_timeslice_element_read.cpp + bench_aos_traversal.cpp bench_main.cpp ) target_link_libraries( al_benchmarks PRIVATE al_contract_fixtures benchmark::benchmark ) add_dependencies( al_benchmarks al ) diff --git a/benchmarks/bench_aos_traversal.cpp b/benchmarks/bench_aos_traversal.cpp new file mode 100644 index 00000000..5d99f27f --- /dev/null +++ b/benchmarks/bench_aos_traversal.cpp @@ -0,0 +1,107 @@ +// AOS-heavy full-traversal benchmark (issue #61, PRD #57 user story 7): the +// "before" baseline for P1/P4's round-trip-count reduction (NORTH_STAR.md +// §7) -- and the macro signal that would justify a future P5 per-call +// primitive micro-benchmark (getenv per call, path-string replacement) if +// this comes back slow. +// +// Functionally this is the same traversal bench_equilibrium_read.cpp already +// drives via equilibrium_seed::read_back, isolated and named as its own +// operation (PRD #57 Implementation Decisions: "AOS-heavy traversal +// benchmark") because its coverage target -- P1/P4 round-trip count -- is +// distinct from "read everything in the IDS" as a whole. Where +// BM_EquilibriumWholeIdsRead also reads the top-level scalar and timebase +// leaves, BM_AosHeavyTraversal scopes strictly to the time_slice +// array-of-structures: al_begin_arraystruct_action, then for every element +// (all equilibrium_seed::kNSlices of them, not just one -- that's +// BM_TimeSliceElementRead's job) read all five leaves and +// al_iterate_over_arraystruct to the next element. +// +// Reuses equilibrium_seed.h verbatim (issue #51); no size parametrization +// (PRD #57 user story 9). +// +// Timed region = one operation (issue #53, PRD #50 user story 6): the +// reference equilibrium is written and the data entry opened once in untimed +// setup; only begin_global_action(READ) -> begin_arraystruct_action -> +// iterate every element, reading every leaf -> end_action(aos) -> +// end_action(op) is inside the `for (auto _ : state)` loop. +// +// Backend is a sub-axis of the operation (PRD #50 "structure: +// scenario-primary, backend as a sub-axis"): BM_AosHeavyTraversal is the one +// operation body, HDF5 and Memory are two BENCHMARK_CAPTURE registrations of +// it differing only in which backend id the URI is built for. + +#include "bench_common.h" +#include "al_contract_abi.h" +#include "al_contract_legacy_storage.h" +#include "equilibrium_seed.h" + +#include +#include + +#include + +using al_bench::BenchBackend; +using al_bench::CheckOk; + +void BM_AosHeavyTraversal(benchmark::State& state, BenchBackend backend) { + // pulse=1, matching bench_equilibrium_read.cpp and + // bench_timeslice_element_read.cpp: all three are read-only operations, so + // (unlike bench_equilibrium_write.cpp's pulse=2) there's no repeated-write + // staleness concern that would require a distinct identity within the same + // al_benchmarks process. + const al_contract::PulseId pulse{/*database=*/"bench", /*version=*/"3", + /*pulse=*/1, /*run=*/0}; + + // --- untimed setup: open once, write the reference equilibrium once ------ + al_bench::BenchPulse bp = al_bench::OpenBenchPulse(backend, pulse); + CheckOk(equilibrium_seed::write(bp.pulse_ctx), "equilibrium_seed::write"); + + // --- timed region: begin_global_action(READ) -> every time_slice element, + // every field, via al_iterate_over_arraystruct -> end_action --------- + std::vector shape; + std::vector data; + for (auto _ : state) { + int op = -1; + CheckOk(al_begin_global_action(bp.pulse_ctx, equilibrium_seed::kIds, "", + READ_OP, &op), + "al_begin_global_action"); + + int size = 0; + int aos_ctx = -1; + CheckOk(al_begin_arraystruct_action(op, equilibrium_seed::kAos, + equilibrium_seed::kAosTime, &size, + &aos_ctx), + "al_begin_arraystruct_action"); + + auto read_leaf = [&](const char* path, int rank, const char* what) { + CheckOk(al_contract::read_data(aos_ctx, path, rank, &shape, &data), + what); + benchmark::DoNotOptimize(data); + }; + for (int i = 0; i < size; ++i) { + read_leaf(equilibrium_seed::kSliceTime, 0, "read time"); + read_leaf(equilibrium_seed::kPsi, 1, "read profiles_1d/psi"); + read_leaf(equilibrium_seed::kIp, 0, "read global_quantities/ip"); + read_leaf(equilibrium_seed::kMeasured, 0, "read constraints/ip/measured"); + read_leaf(equilibrium_seed::kWeight, 0, "read constraints/ip/weight"); + if (i + 1 < size) { + CheckOk(al_iterate_over_arraystruct(aos_ctx, 1), + "al_iterate_over_arraystruct"); + } + } + + CheckOk(al_end_action(aos_ctx), "al_end_action(aos_ctx)"); + CheckOk(al_end_action(op), "al_end_action(op)"); + } + + // --- untimed teardown ------------------------------------------------------ + CheckOk(al_close_pulse(bp.pulse_ctx, CLOSE_PULSE), "al_close_pulse"); + // bp.base's destructor (HDF5 only) removes the temp directory on every exit path. +} +// Repetitions(N) makes Google Benchmark report the distribution (median, +// mean, stddev, iteration count) across N independent traversal cycles, not +// a bare single-shot mean (issue #53, PRD #50 user story 8). +BENCHMARK_CAPTURE(BM_AosHeavyTraversal, HDF5, BenchBackend::kHdf5) + ->Repetitions(20); +BENCHMARK_CAPTURE(BM_AosHeavyTraversal, Memory, BenchBackend::kMemory) + ->Repetitions(20); From 845ab9f5a278a33480de71ffc1951d2c04b5f393 Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Tue, 21 Jul 2026 17:22:50 +0200 Subject: [PATCH 12/12] Address issue 57 review findings (cherry picked from commit e41a9440227e3fca7d034387113a3649a15a235b) --- benchmarks/bench_aos_traversal.cpp | 20 +++-------- benchmarks/bench_equilibrium_traversal.h | 35 +++++++++++++++++++ benchmarks/bench_equilibrium_write.cpp | 37 ++------------------- benchmarks/bench_timeslice_element_read.cpp | 16 ++------- 4 files changed, 44 insertions(+), 64 deletions(-) create mode 100644 benchmarks/bench_equilibrium_traversal.h diff --git a/benchmarks/bench_aos_traversal.cpp b/benchmarks/bench_aos_traversal.cpp index 5d99f27f..ff6841fa 100644 --- a/benchmarks/bench_aos_traversal.cpp +++ b/benchmarks/bench_aos_traversal.cpp @@ -1,7 +1,7 @@ // AOS-heavy full-traversal benchmark (issue #61, PRD #57 user story 7): the // "before" baseline for P1/P4's round-trip-count reduction (NORTH_STAR.md -// §7) -- and the macro signal that would justify a future P5 per-call -// primitive micro-benchmark (getenv per call, path-string replacement) if +// §7) -- and the macro signal that would justify a future P5 Primitive +// benchmark (getenv per call, path-string replacement) if // this comes back slow. // // Functionally this is the same traversal bench_equilibrium_read.cpp already @@ -30,10 +30,7 @@ // operation body, HDF5 and Memory are two BENCHMARK_CAPTURE registrations of // it differing only in which backend id the URI is built for. -#include "bench_common.h" -#include "al_contract_abi.h" -#include "al_contract_legacy_storage.h" -#include "equilibrium_seed.h" +#include "bench_equilibrium_traversal.h" #include #include @@ -73,17 +70,8 @@ void BM_AosHeavyTraversal(benchmark::State& state, BenchBackend backend) { &aos_ctx), "al_begin_arraystruct_action"); - auto read_leaf = [&](const char* path, int rank, const char* what) { - CheckOk(al_contract::read_data(aos_ctx, path, rank, &shape, &data), - what); - benchmark::DoNotOptimize(data); - }; for (int i = 0; i < size; ++i) { - read_leaf(equilibrium_seed::kSliceTime, 0, "read time"); - read_leaf(equilibrium_seed::kPsi, 1, "read profiles_1d/psi"); - read_leaf(equilibrium_seed::kIp, 0, "read global_quantities/ip"); - read_leaf(equilibrium_seed::kMeasured, 0, "read constraints/ip/measured"); - read_leaf(equilibrium_seed::kWeight, 0, "read constraints/ip/weight"); + al_bench::ReadCurrentEquilibriumTimeSliceLeaves(aos_ctx, &shape, &data); if (i + 1 < size) { CheckOk(al_iterate_over_arraystruct(aos_ctx, 1), "al_iterate_over_arraystruct"); diff --git a/benchmarks/bench_equilibrium_traversal.h b/benchmarks/bench_equilibrium_traversal.h new file mode 100644 index 00000000..e4b3e6b8 --- /dev/null +++ b/benchmarks/bench_equilibrium_traversal.h @@ -0,0 +1,35 @@ +// Shared equilibrium time_slice leaf traversal for the operation benchmarks. +// Keeping the fixture-specific path list here prevents the single-element and +// full-AOS benchmarks from drifting while leaving their iteration boundaries +// and timed operation structure explicit at each call site. +#ifndef AL_BENCHMARKS_EQUILIBRIUM_TRAVERSAL_H +#define AL_BENCHMARKS_EQUILIBRIUM_TRAVERSAL_H + +#include "al_contract_abi.h" +#include "bench_common.h" +#include "equilibrium_seed.h" + +#include + +#include + +namespace al_bench { + +inline void ReadCurrentEquilibriumTimeSliceLeaves( + int aos_ctx, std::vector* shape, std::vector* data) { + auto read_leaf = [&](const char* path, int rank, const char* what) { + CheckOk(al_contract::read_data(aos_ctx, path, rank, shape, data), + what); + benchmark::DoNotOptimize(*data); + }; + + read_leaf(equilibrium_seed::kSliceTime, 0, "read time"); + read_leaf(equilibrium_seed::kPsi, 1, "read profiles_1d/psi"); + read_leaf(equilibrium_seed::kIp, 0, "read global_quantities/ip"); + read_leaf(equilibrium_seed::kMeasured, 0, "read constraints/ip/measured"); + read_leaf(equilibrium_seed::kWeight, 0, "read constraints/ip/weight"); +} + +} // namespace al_bench + +#endif // AL_BENCHMARKS_EQUILIBRIUM_TRAVERSAL_H diff --git a/benchmarks/bench_equilibrium_write.cpp b/benchmarks/bench_equilibrium_write.cpp index 2e9b405c..f8b5c5c1 100644 --- a/benchmarks/bench_equilibrium_write.cpp +++ b/benchmarks/bench_equilibrium_write.cpp @@ -10,23 +10,9 @@ // `for (auto _ : state)` loop, so open/close cost never contaminates the // write measurement. // -// Repeated writes to the same open pulse context must never accumulate stale -// data from a prior write (PRD #57 user story 5): ALData::writeData clears -// its buffer before repopulating (src/memory_backend.cpp) and the -// array-of-structures path replaces rather than appends elements, for any -// whole-IDS (non-slice) write. This is checked once, untimed, after the timed -// loop -- but the timed loop's own iteration count isn't a reliable way to -// exercise "repeated": the CTest smoke entry caps every benchmark at exactly -// one iteration (--benchmark_min_time=1x), which would make the check -// vacuous (one write trivially matches one write's hash) on the one run this -// repo executes automatically. So an explicit second untimed write happens -// after the loop, guaranteeing at least two writes to the same context -// regardless of how many timed iterations ran, before a read-back of the -// resulting structural hash is compared against equilibrium_seed's -// expected_hash() exactly, on both backends. The hash folds in AOS size -// (equilibrium_seed.h), so an accumulation bug (e.g. elements appended -// instead of replaced) would drift it and this check -- which the existing -// CTest smoke entry already runs -- would throw. +// Correctness of repeated writes on the same open pulse is pinned separately +// by EquilibriumSeedMatrix.RepeatedWholeIdsWriteOnSameOpenPulseSucceeds; this +// benchmark measures the operation without re-asserting the contract. #include "bench_common.h" #include "equilibrium_seed.h" @@ -36,9 +22,6 @@ #include -#include -#include - using al_bench::BenchBackend; using al_bench::CheckOk; @@ -57,20 +40,6 @@ void BM_EquilibriumWholeIdsWrite(benchmark::State& state, BenchBackend backend) CheckOk(equilibrium_seed::write(bp.pulse_ctx), "equilibrium_seed::write"); } - // --- untimed correctness check: no stale data survives repeated writes --- - // Force a second write regardless of the timed loop's iteration count (see - // top-of-file comment), then verify no staleness accumulated. - CheckOk(equilibrium_seed::write(bp.pulse_ctx), "equilibrium_seed::write"); - uint64_t hash = 0; - CheckOk(equilibrium_seed::read_and_hash(bp.pulse_ctx, &hash), - "equilibrium_seed::read_and_hash"); - if (hash != equilibrium_seed::expected_hash()) { - throw std::runtime_error( - "BM_EquilibriumWholeIdsWrite: repeated-write structural hash does " - "not match expected_hash() -- repeated writes may have accumulated " - "stale data"); - } - // --- untimed teardown ------------------------------------------------------ CheckOk(al_close_pulse(bp.pulse_ctx, CLOSE_PULSE), "al_close_pulse"); // bp.base's destructor (HDF5 only) removes the temp directory on every exit path. diff --git a/benchmarks/bench_timeslice_element_read.cpp b/benchmarks/bench_timeslice_element_read.cpp index 90c5cf80..04e1710d 100644 --- a/benchmarks/bench_timeslice_element_read.cpp +++ b/benchmarks/bench_timeslice_element_read.cpp @@ -29,10 +29,7 @@ // registrations of it differing only in which backend id the URI is built // for. -#include "bench_common.h" -#include "al_contract_abi.h" -#include "al_contract_legacy_storage.h" -#include "equilibrium_seed.h" +#include "bench_equilibrium_traversal.h" #include #include @@ -74,16 +71,7 @@ void BM_TimeSliceElementRead(benchmark::State& state, BenchBackend backend) { CheckOk(al_iterate_over_arraystruct(aos_ctx, kTargetSlice), "al_iterate_over_arraystruct"); - auto read_leaf = [&](const char* path, int rank, const char* what) { - CheckOk(al_contract::read_data(aos_ctx, path, rank, &shape, &data), - what); - benchmark::DoNotOptimize(data); - }; - read_leaf(equilibrium_seed::kSliceTime, 0, "read time"); - read_leaf(equilibrium_seed::kPsi, 1, "read profiles_1d/psi"); - read_leaf(equilibrium_seed::kIp, 0, "read global_quantities/ip"); - read_leaf(equilibrium_seed::kMeasured, 0, "read constraints/ip/measured"); - read_leaf(equilibrium_seed::kWeight, 0, "read constraints/ip/weight"); + al_bench::ReadCurrentEquilibriumTimeSliceLeaves(aos_ctx, &shape, &data); CheckOk(al_end_action(aos_ctx), "al_end_action(aos_ctx)"); CheckOk(al_end_action(op), "al_end_action(op)");