From 382f2caba3a07f904b420acbc2ce210e0ef81835 Mon Sep 17 00:00:00 2001 From: James Xin Date: Fri, 21 Aug 2026 10:11:40 -0700 Subject: [PATCH] Build each engine once per sweep instead of once per matrix cell Signed-off-by: James Xin --- Makefile | 70 ++++++++---- README.md | 12 ++- docs/ADDING_LANGUAGE.md | 24 +++-- docs/BENCHMARK_MATRIX.md | 15 ++- scripts/run_benchmark_matrix.py | 55 ++++++---- scripts/tests/test_engine_build.py | 101 ++++++++++++++++++ .../tests/test_matrix_failure_visibility.py | 6 ++ 7 files changed, 235 insertions(+), 48 deletions(-) create mode 100644 scripts/tests/test_engine_build.py diff --git a/Makefile b/Makefile index db7d5d2..0748156 100644 --- a/Makefile +++ b/Makefile @@ -30,10 +30,10 @@ WORK_DIR=$(shell pwd)/work server-cluster-start server-cluster-stop server-cluster-init \ server-sentinel-start server-sentinel-stop \ server-start server-stop \ - java-build java-test java-run java-clean \ + java-build java-test java-run java-run-nobuild java-clean \ python-build python-test python-run python-clean \ - ruby-build ruby-test ruby-run ruby-clean ruby-info \ - csharp-build csharp-test csharp-run csharp-clean csharp-info \ + ruby-build ruby-test ruby-run ruby-run-nobuild ruby-clean ruby-info \ + csharp-build csharp-test csharp-run csharp-run-nobuild csharp-clean csharp-info \ config-editor-build config-editor-dev # ============================================================================ @@ -54,6 +54,7 @@ help: @echo " make java-build Build Java benchmark engine" @echo " make java-test Run Java tests" @echo " make java-run Run Java benchmark (requires DRIVER and WORKLOAD)" + @echo " make java-run-nobuild Run Java benchmark without rebuilding first" @echo " make java-clean Clean Java build artifacts" @echo "" @echo "Python Engine (placeholder):" @@ -65,12 +66,14 @@ help: @echo " make ruby-build Install Ruby dependencies" @echo " make ruby-test Run Ruby tests" @echo " make ruby-run Run Ruby benchmark (requires DRIVER and WORKLOAD)" + @echo " make ruby-run-nobuild Run Ruby benchmark without re-running bundle install" @echo " make ruby-info Show supported Ruby drivers and commands" @echo "" @echo "C# Engine:" @echo " make csharp-build Build C# benchmark engine" @echo " make csharp-test Run C# tests" @echo " make csharp-run Run C# benchmark (requires DRIVER and WORKLOAD)" + @echo " make csharp-run-nobuild Run C# benchmark without rebuilding first" @echo " make csharp-clean Clean C# build artifacts" @echo " make csharp-info Show supported C# drivers and commands" @echo "" @@ -284,12 +287,24 @@ java-integration-test: server-standalone-start cd java && VALKEY_HOST=localhost VALKEY_PORT=6379 mvn test -DincludeIntegrationTests $(MAKE) server-standalone-stop +# The run command is held in a variable so `java-run` and `java-run-nobuild` +# cannot drift apart. Deliberately not `java-run: java-build java-run-nobuild`: +# prerequisite ordering is not guaranteed under `make -j`, and a benchmark that +# silently runs a stale jar is worse than a duplicated prerequisite. +JAVA_RUN_CMD=java -jar $(JAVA_JAR) \ + --server $(SERVER) \ + --driver $(DRIVER) \ + --workload $(WORKLOAD) \ + --metrics $(METRICS_OUTPUT) + java-run: java-build - java -jar $(JAVA_JAR) \ - --server $(SERVER) \ - --driver $(DRIVER) \ - --workload $(WORKLOAD) \ - --metrics $(METRICS_OUTPUT) + $(JAVA_RUN_CMD) + +# Same as java-run but assumes the engine is already built. Used by the matrix +# orchestrator, which builds each engine it needs once per sweep instead of +# once per cell. +java-run-nobuild: + $(JAVA_RUN_CMD) java-run-cluster: java-build java -jar $(JAVA_JAR) \ @@ -342,12 +357,18 @@ ruby-integration-test: server-standalone-start cd ruby && VALKEY_HOST=localhost VALKEY_PORT=6379 bundle exec rake integration $(MAKE) server-standalone-stop +RUBY_RUN_CMD=cd ruby && bundle exec ruby bin/resp-bench \ + --server $(SERVER) \ + --driver ../$(DRIVER) \ + --workload ../$(WORKLOAD) \ + --metrics ../$(METRICS_OUTPUT) + ruby-run: ruby-build - cd ruby && bundle exec ruby bin/resp-bench \ - --server $(SERVER) \ - --driver ../$(DRIVER) \ - --workload ../$(WORKLOAD) \ - --metrics ../$(METRICS_OUTPUT) + $(RUBY_RUN_CMD) + +# Same as ruby-run but skips `bundle install` — see java-run-nobuild. +ruby-run-nobuild: + $(RUBY_RUN_CMD) ruby-clean: cd ruby && rm -rf vendor .bundle Gemfile.lock @@ -372,12 +393,21 @@ csharp-integration-test: server-standalone-start cd csharp && VALKEY_HOST=localhost VALKEY_PORT=6379 dotnet test --filter "Category=Integration" $(MAKE) server-standalone-stop +# Only the arguments are shared here, not the whole command: unlike java and +# ruby, the two C# recipes genuinely differ — `dotnet run` builds by default, so +# the nobuild variant has to pass --no-build. +CSHARP_RUN_ARGS=--server $(SERVER) \ + --driver $(DRIVER) \ + --workload $(WORKLOAD) \ + --metrics $(METRICS_OUTPUT) + csharp-run: csharp-build - dotnet run --project $(CSHARP_PROJECT) -c Release -- \ - --server $(SERVER) \ - --driver $(DRIVER) \ - --workload $(WORKLOAD) \ - --metrics $(METRICS_OUTPUT) + dotnet run --project $(CSHARP_PROJECT) -c Release -- $(CSHARP_RUN_ARGS) + +# Same as csharp-run but assumes the engine is already built, so `dotnet run` +# skips its own incremental build too — see java-run-nobuild. +csharp-run-nobuild: + dotnet run --project $(CSHARP_PROJECT) -c Release --no-build -- $(CSHARP_RUN_ARGS) csharp-clean: cd csharp && dotnet clean @@ -408,7 +438,9 @@ GRAPHS_DIR?=graphs/interactive/ RUN_ID?=latest MATRIX_RESULTS_DIR=$(OUTPUT_DIR)/$(RUN_ID) -benchmark-matrix: java-build +# No build prerequisite: the orchestrator builds every engine the matrix needs +# (and only those) once, before the sweep starts. +benchmark-matrix: python scripts/run_benchmark_matrix.py \ --matrix $(MATRIX) \ --output-dir $(OUTPUT_DIR) \ diff --git a/README.md b/README.md index 92e3af7..f52043c 100644 --- a/README.md +++ b/README.md @@ -198,12 +198,18 @@ See [docs/CONFIG_SPECIFICATION.md](docs/CONFIG_SPECIFICATION.md) for full detail | Target | Description | |--------|-------------| -| `make java-run` | Run Java engine (DRIVER, WORKLOAD, SERVER) | -| `make ruby-run` | Run Ruby engine (DRIVER, WORKLOAD, SERVER) | -| `make csharp-run` | Run C# engine (DRIVER, WORKLOAD, SERVER) | +| `make java-run` | Build, then run Java engine (DRIVER, WORKLOAD, SERVER) | +| `make ruby-run` | Build, then run Ruby engine (DRIVER, WORKLOAD, SERVER) | +| `make csharp-run` | Build, then run C# engine (DRIVER, WORKLOAD, SERVER) | +| `make java-run-nobuild` | Run Java engine without rebuilding (used by the matrix orchestrator) | +| `make ruby-run-nobuild` | Run Ruby engine without re-running `bundle install` | +| `make csharp-run-nobuild` | Run C# engine without rebuilding | | `make java-build` | Build Java JAR | +| `make ruby-build` | Install Ruby dependencies | | `make csharp-build` | Build C# executable | +The matrix orchestrator builds each engine the matrix needs once per sweep and then uses the `*-run-nobuild` targets per cell — see [docs/BENCHMARK_MATRIX.md](docs/BENCHMARK_MATRIX.md#engine-builds--once-per-sweep). + ### Server Management | Target | Description | diff --git a/docs/ADDING_LANGUAGE.md b/docs/ADDING_LANGUAGE.md index c8adab4..4ce700e 100644 --- a/docs/ADDING_LANGUAGE.md +++ b/docs/ADDING_LANGUAGE.md @@ -294,14 +294,26 @@ python-build: python-test: cd python && pytest +PYTHON_RUN_CMD=python -m resp_bench \ + --server $(SERVER) \ + --driver $(DRIVER) \ + --workload $(WORKLOAD) \ + --metrics $(METRICS_OUTPUT) + python-run: python-build - python -m resp_bench \ - --server $(SERVER) \ - --driver $(DRIVER) \ - --workload $(WORKLOAD) \ - --metrics $(METRICS_OUTPUT) + $(PYTHON_RUN_CMD) + +python-run-nobuild: + $(PYTHON_RUN_CMD) ``` +Both run targets are required. The matrix orchestrator builds each engine once +per sweep and then invokes `-run-nobuild` per cell, so an engine that +only defines `-run` either rebuilds on every cell or — once its +`driver_id` is registered in `DRIVER_ENGINE_MAP` — fails every cell with +`No rule to make target`. See +[BENCHMARK_MATRIX.md](BENCHMARK_MATRIX.md#engine-builds--once-per-sweep). + ### 11. Implement Client Drivers For each driver (e.g., redis-py): @@ -395,7 +407,7 @@ Before submitting a new language engine: - [ ] All unit tests pass - [ ] Integration tests pass against live server - [ ] Documentation complete -- [ ] Makefile targets work correctly +- [ ] Makefile targets work correctly, including `-run-nobuild` ## Cross-Language Validation diff --git a/docs/BENCHMARK_MATRIX.md b/docs/BENCHMARK_MATRIX.md index ca0bb16..3f840f6 100644 --- a/docs/BENCHMARK_MATRIX.md +++ b/docs/BENCHMARK_MATRIX.md @@ -38,6 +38,19 @@ make benchmark-matrix-graphs OUTPUT_DIR=results/glide-sweep make benchmark-matrix-graphs OUTPUT_DIR=results/glide-sweep RUN_ID=20260321T140322Z ``` +## Engine Builds — Once Per Sweep + +Before the sweep starts, the orchestrator resolves the engine behind every `driver_config` (via the driver's `driver_id`) and runs `make -build` **once for each engine the matrix actually needs**. A Java-only matrix builds Java only; a mixed matrix builds Java, Ruby and C#. If a build fails the run aborts immediately, rather than failing every cell. + +Individual cells then execute `make -run-nobuild`, which runs the already-built engine without rebuilding it. This matters because sweeps are large — `driver-comparison-high-tps` is 720 cells — and `*-build` is not incremental (`mvn clean package`, `bundle install`, `dotnet build -c Release`). + +| Target | Behavior | +|--------|----------| +| `make java-run` / `ruby-run` / `csharp-run` | Build, then run. Unchanged — the right target for one-off manual runs. | +| `make java-run-nobuild` / `ruby-run-nobuild` / `csharp-run-nobuild` | Run only. Assumes the engine is already built; used by the matrix orchestrator. | + +`make benchmark-matrix` no longer pre-builds Java itself, since the orchestrator builds exactly the engines the chosen matrix requires. + ## Matrix Config Format Matrix configs live in `configs/matrices/` and define **dimensions** to sweep: @@ -234,4 +247,4 @@ python scripts/run_benchmark_matrix.py --help `GLIDE_TOKIO_WORKER_THREADS` and `GLIDE_CALLBACK_WORKER_THREADS` are **process-level environment variables** consumed by the native Rust/Tokio runtime inside the valkey-glide JAR. They are read once when `GlideClient.createClient()` first initializes the process-wide Tokio runtime, and cannot be changed afterward. -Because the matrix runner launches each benchmark as a separate JVM process (via `make java-run`), different env var values can be set per run. These are specified in the matrix config's `env` dimension, NOT in the driver config JSON. +Because the matrix runner launches each benchmark as a separate JVM process (via `make java-run-nobuild`), different env var values can be set per run. These are specified in the matrix config's `env` dimension, NOT in the driver config JSON. diff --git a/scripts/run_benchmark_matrix.py b/scripts/run_benchmark_matrix.py index 4bfcf9d..3f2837e 100644 --- a/scripts/run_benchmark_matrix.py +++ b/scripts/run_benchmark_matrix.py @@ -722,23 +722,30 @@ def count_ndjson_lines(path, offset=0): return sum(1 for line in f if line.strip()) -def run_benchmark(server, driver_file, workload_file, metrics_output, env_overrides=None): - """Run a single benchmark via `make java-run`.""" - env = os.environ.copy() - if env_overrides: - env.update({k: str(v) for k, v in env_overrides.items()}) +def engines_for_combos(series_combos): + """Map each distinct driver config path to the engine that runs it. + + Each config file is read once, however many cells reference it. + """ + driver_configs = {combo["driver_config"] for combo in series_combos} + return {path: detect_engine_for_driver(path) for path in driver_configs} - subprocess.run( - [ - "make", "java-run", - f"SERVER={server}", - f"DRIVER={driver_file}", - f"WORKLOAD={workload_file}", - f"METRICS_OUTPUT={metrics_output}", - ], - check=True, - env=env, - ) + +def build_engines(engines): + """Run `make -build` for each engine, aborting on the first failure. + + Cells run via the `*-run-nobuild` targets, so a sweep builds each engine it + needs exactly once instead of once per cell. + """ + for engine in engines: + target = f"{engine}-build" + print(f"\n=== building engine: make {target} ===", flush=True) + result = subprocess.run(["make", target]) + if result.returncode != 0: + sys.exit( + f"ERROR: 'make {target}' failed with exit code " + f"{result.returncode}; aborting matrix run" + ) # ═══════════════════════════════════════════════════════════════════════════════ @@ -921,6 +928,11 @@ def run_matrix(config, output_dir, server_host, port, run_id=None, resume=False, run_id = run_id or default_run_id() + # Resolve the engine per driver config up front, so the build phase and the + # per-cell run phase can never disagree about which engine a series uses. + engine_by_driver = engines_for_combos(series_combos) + engines = sorted(set(engine_by_driver.values())) + print("=" * 70) print(f"Matrix Benchmark Run") print(f" Description: {config['description']}") @@ -936,6 +948,7 @@ def run_matrix(config, output_dir, server_host, port, run_id=None, resume=False, print(f" bindings: {combo['bindings']}") print(f" Iterations: {iterations}") print(f" Total runs: {total_cells}") + print(f" Engines: {', '.join(engines)}") print("=" * 70) # Preflight — refuse to start rather than dying part-way through the sweep @@ -953,6 +966,10 @@ def run_matrix(config, output_dir, server_host, port, run_id=None, resume=False, if latest_link: print(f"Preflight: {latest_link} -> {run_id}") + # Build each needed engine exactly once, now that preflight has confirmed the + # server is reachable — no point compiling if the sweep can't run. + build_engines(engines) + # Write manifest write_manifest(results_dir, config, series_combos, run_id=run_id, resumed=resume) @@ -1016,10 +1033,10 @@ def run_matrix(config, output_dir, server_host, port, run_id=None, resume=False, if env_overrides: bench_env.update({k: str(v) for k, v in env_overrides.items()}) - # Auto-detect engine from driver config - engine = detect_engine_for_driver(str(driver_path)) + # Engine was resolved and built before the sweep started + engine = engine_by_driver[driver_cfg_path] bench_cmd = [ - "make", f"{engine}-run", + "make", f"{engine}-run-nobuild", f"SERVER={server}", f"DRIVER={str(driver_path)}", f"WORKLOAD={str(workload_path)}", diff --git a/scripts/tests/test_engine_build.py b/scripts/tests/test_engine_build.py new file mode 100644 index 0000000..4bba52e --- /dev/null +++ b/scripts/tests/test_engine_build.py @@ -0,0 +1,101 @@ +"""Tests for build-once-per-sweep engine resolution and building.""" +import json +import re +import subprocess +from pathlib import Path + +import pytest + +from run_benchmark_matrix import DRIVER_ENGINE_MAP, build_engines, engines_for_combos + +MAKEFILE = Path(__file__).parent.parent.parent / "Makefile" + + +def write_driver(tmp_path, name, driver_id): + p = tmp_path / f"{name}.json" + p.write_text(json.dumps({"driver_id": driver_id, "mode": "standalone"})) + return str(p) + + +def combo(driver_config, label="s"): + return {"label": label, "driver_config": driver_config, "params": {}, "bindings": {}} + + +@pytest.fixture +def stub_make(monkeypatch): + """Replace subprocess.run with a stub; returns the list of commands it saw.""" + def install(returncode=0): + calls = [] + + def fake_run(cmd, *args, **kwargs): + calls.append(cmd) + return subprocess.CompletedProcess(cmd, returncode) + + monkeypatch.setattr(subprocess, "run", fake_run) + return calls + + return install + + +class TestEnginesForCombos: + def test_one_entry_per_distinct_driver_config(self, tmp_path): + jedis = write_driver(tmp_path, "jedis", "jedis") + combos = [combo(jedis, f"pool={n}") for n in (8, 16, 32)] + + assert engines_for_combos(combos) == {jedis: "java"} + + def test_mixed_matrix_needs_every_engine(self, tmp_path): + drivers = [ + write_driver(tmp_path, "jedis", "jedis"), + write_driver(tmp_path, "redis-rb", "redis-rb"), + write_driver(tmp_path, "se-redis", "stackexchange-redis"), + ] + engine_by_driver = engines_for_combos([combo(d, d) for d in drivers]) + + assert sorted(set(engine_by_driver.values())) == ["csharp", "java", "ruby"] + + def test_unknown_driver_id_falls_back_to_java(self, tmp_path): + unknown = write_driver(tmp_path, "unknown", "no-such-driver") + + assert engines_for_combos([combo(unknown)]) == {unknown: "java"} + + +class TestBuildEngines: + def test_builds_each_engine_once(self, stub_make): + calls = stub_make() + + build_engines(["csharp", "java"]) + + assert calls == [["make", "csharp-build"], ["make", "java-build"]] + + def test_java_only_matrix_does_not_build_dotnet_or_ruby(self, tmp_path, stub_make): + """A Java-only matrix must not pay for the .NET or Ruby build.""" + combos = [ + combo(write_driver(tmp_path, "jedis", "jedis"), "jedis"), + combo(write_driver(tmp_path, "lettuce", "lettuce"), "lettuce"), + ] + calls = stub_make() + + build_engines(sorted(set(engines_for_combos(combos).values()))) + + assert calls == [["make", "java-build"]] + + def test_build_failure_aborts_before_running_later_engines(self, stub_make): + calls = stub_make(returncode=2) + + with pytest.raises(SystemExit) as excinfo: + build_engines(["java", "ruby"]) + + assert calls == [["make", "java-build"]] + assert "java-build" in str(excinfo.value) + + +class TestMakefileContract: + """Every engine the orchestrator can dispatch to needs both make targets.""" + + @pytest.mark.parametrize("engine", sorted(set(DRIVER_ENGINE_MAP.values()))) + def test_engine_has_build_and_run_nobuild_targets(self, engine): + makefile = MAKEFILE.read_text() + for target in (f"{engine}-build", f"{engine}-run-nobuild"): + assert re.search(rf"^{re.escape(target)}\s*:", makefile, re.MULTILINE), \ + f"Makefile has no '{target}' rule, but DRIVER_ENGINE_MAP maps a driver to '{engine}'" diff --git a/scripts/tests/test_matrix_failure_visibility.py b/scripts/tests/test_matrix_failure_visibility.py index 3e56ea6..d18a31a 100644 --- a/scripts/tests/test_matrix_failure_visibility.py +++ b/scripts/tests/test_matrix_failure_visibility.py @@ -466,10 +466,16 @@ def stub_engine(monkeypatch, returncode=0, records_written=0): Only `make -run` invocations are stubbed; anything else (the CLI used by the readiness probe) still runs for real. + + The once-per-sweep engine build (`build_engines`, which shells `make + -build`) is stubbed to a no-op here — these tests exercise run_matrix + outcomes, and the build step is covered separately in test_engine_build.py. """ calls = [] real_popen = subprocess.Popen + monkeypatch.setattr("run_benchmark_matrix.build_engines", lambda engines: None) + def fake_popen(cmd, **kwargs): metrics_args = [a for a in cmd if str(a).startswith("METRICS_OUTPUT=")] if not metrics_args: