Skip to content

Commit cb3bd69

Browse files
authored
GH-49913: [Archery] Add preserve-dir and improve directory layout (#50056)
### Rationale for this change Improve workflow when running many benchmarks. Namely eagerly store results and avoid user having to manage large amount of file names. ### What changes are included in this PR? With the following code run **twice**, ```bash archery benchmark run --suite-filter=parquet --benchmark-filter=Decode --preserve --output out.json HEAD --preserve --preserve-dir bench/ ``` The `preserve-dir` is used and its internal structure is as follows. ``` bench └── 0202ab5295fee14c6c7b85b05ca78cc5c281b3a5 ├── arrow │   └── // Source files... ├── bench │   ├── 84e0ab99 │  │   ├── benchmark.json │   │   └── metadata.json │   └── e15e438a │   ├── benchmark.json │   └── metadata.json └── build ├── 84e0ab99 │   └── // Cmake build files... └── e15e438a └── // Cmake build files... ``` The commit is uniquerly resolved source is reused between runs. Separate invocation results in separate builds and benchmark execution. The idea would be to be able to reuse the build dir but it is tricky to tell when it should be changed (If CLI params are different? If environment var change? If dependencies change?). The idea is to get started minimally without to much maintenance. Metadata are also a starting point to avoid forgetting what a (costly!) run was about. Example from current run: ```json { "time": "2026-05-27T13:58:02.792064+00:00", "cmd": { "full": "archery benchmark run --suite-filter=parquet --benchmark-filter=Decode --preserve --output out.json HEAD --preserve --preserve-dir bench/", "params": { "suite_filter": "parquet", "benchmark_filter": "Decode", "preserve": true, "output": "<unopened file 'out.json' w>", "preserve_dir": ".../arrow/bench", "rev_or_path": "HEAD", "cpp_benchmark_extras": [], "cmake_extras": [], "benchmark_extras": [], "build_extras": [], "language": "cpp", "src": ".../arrow", "cpp_package_prefix": null, "cxx_flags": null, "cxx": null, "cc": null, "java_options": null, "java_home": null, "repetitions": -1, "repetition_min_time": null } }, "machine_info": { "platform": "macOS-26.5-arm64-arm-64bit-Mach-O", "system": "Darwin", "release": "25.5.0", "version": "Darwin Kernel Version 25.5.0: Mon Apr 27 20:41:06 PDT 2026; root:xnu-12377.121.6~2/RELEASE_ARM64_T6030", "machine": "arm64", "processor": "arm", "architecture": "64bit", "logical_cores": 12 } } ``` ### Are these changes tested? Yes ### Are there any user-facing changes? Archery user that looked into the `--preserve` directory. * GitHub Issue: #49913 Authored-by: AntoinePrv <AntoinePrv@users.noreply.github.com> Signed-off-by: Antoine Pitrou <antoine@python.org>
1 parent 72a970f commit cb3bd69

3 files changed

Lines changed: 125 additions & 35 deletions

File tree

dev/archery/archery/benchmark/runner.py

Lines changed: 58 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,19 +15,42 @@
1515
# specific language governing permissions and limitations
1616
# under the License.
1717

18+
import datetime
1819
import glob
1920
import json
2021
import os
22+
import random
2123
import re
24+
import subprocess
2225

2326
from .core import BenchmarkSuite
2427
from .google import GoogleBenchmarkCommand, GoogleBenchmark
2528
from .jmh import JavaMicrobenchmarkHarnessCommand, JavaMicrobenchmarkHarness
2629
from ..lang.cpp import CppCMakeDefinition, CppConfiguration
2730
from ..lang.java import JavaMavenDefinition, JavaConfiguration
2831
from ..utils.cmake import CMakeBuild
32+
from ..utils.git import git
2933
from ..utils.maven import MavenBuild
3034
from ..utils.logger import logger
35+
from ..utils.source import ArrowSources
36+
37+
38+
def _rev_or_path_dirname(src, rev_or_path):
39+
"""Return a directory-name-safe identifier for a revision or a path.
40+
41+
If ``rev_or_path`` resolves to a git revision, its full SHA is returned.
42+
Otherwise (e.g. it is a filesystem path), a sanitized form with path
43+
separators replaced is returned.
44+
"""
45+
if rev_or_path == ArrowSources.WORKSPACE:
46+
return rev_or_path
47+
try:
48+
sha = git.rev_parse(rev_or_path, git_dir=src.path)
49+
if isinstance(sha, bytes):
50+
sha = sha.decode("ascii")
51+
return sha
52+
except subprocess.CalledProcessError:
53+
return rev_or_path.replace("/", "_")
3154

3255

3356
def regex_filter(re_expr):
@@ -47,6 +70,7 @@ def __init__(self, suite_filter=None, benchmark_filter=None,
4770
self.benchmark_filter = benchmark_filter
4871
self.repetitions = repetitions
4972
self.repetition_min_time = repetition_min_time
73+
self.results_dir = None
5074

5175
@property
5276
def suites(self):
@@ -108,11 +132,14 @@ def __repr__(self):
108132
class CppBenchmarkRunner(BenchmarkRunner):
109133
""" Run suites from a CMakeBuild. """
110134

111-
def __init__(self, build, benchmark_extras, **kwargs):
135+
def __init__(self, build, benchmark_extras, run_id=None, results_dir=None,
136+
**kwargs):
112137
""" Initialize a CppBenchmarkRunner. """
138+
super().__init__(**kwargs)
113139
self.build = build
114140
self.benchmark_extras = benchmark_extras
115-
super().__init__(**kwargs)
141+
self.run_id = run_id
142+
self.results_dir = results_dir
116143

117144
@staticmethod
118145
def default_configuration(**kwargs):
@@ -217,19 +244,33 @@ def from_rev_or_path(src, root, rev_or_path, cmake_conf, **kwargs):
217244
build = CMakeBuild.from_path(rev_or_path)
218245
return CppBenchmarkRunner(build, **kwargs)
219246
else:
220-
# Revisions can references remote via the `/` character, ensure
221-
# that the revision is path friendly
222-
path_rev = rev_or_path.replace("/", "_")
247+
path_rev = _rev_or_path_dirname(src, rev_or_path)
223248
root_rev = os.path.join(root, path_rev)
224-
os.mkdir(root_rev)
249+
os.makedirs(root_rev, exist_ok=True)
225250

251+
# Clone dir is reused when there is known revision (git sha)
252+
# in <preserve-dir>/sha/arrow
226253
clone_dir = os.path.join(root_rev, "arrow")
227-
# Possibly checkout the sources at given revision, no need to
228-
# perform cleanup on cloned repository as root_rev is reclaimed.
229-
src_rev, _ = src.at_revision(rev_or_path, clone_dir)
254+
if os.path.isdir(clone_dir):
255+
src_rev = ArrowSources(clone_dir)
256+
else:
257+
src_rev, _ = src.at_revision(rev_or_path, clone_dir)
230258
cmake_def = CppCMakeDefinition(src_rev.cpp, cmake_conf)
231-
build_dir = os.path.join(root_rev, "build")
232-
return CppBenchmarkRunner(cmake_def.build(build_dir), **kwargs)
259+
run_root = os.path.join(root_rev, "build")
260+
os.makedirs(run_root, exist_ok=True)
261+
# run_id is a path-safe ISO-8601 UTC timestamp down to the second
262+
# so users get a brief idea of what run they are looking at, plus a
263+
# random suffix to avoid collisions between runs in the same second.
264+
now = datetime.datetime.now(datetime.timezone.utc)
265+
run_id = now.strftime("%Y-%m-%dT%H-%M-%SZ") + \
266+
f"-{random.randrange(16**8):08x}"
267+
build_dir = os.path.join(run_root, run_id)
268+
build = cmake_def.build(build_dir)
269+
results_dir = os.path.join(root_rev, "bench", run_id)
270+
os.makedirs(results_dir, exist_ok=True)
271+
return CppBenchmarkRunner(
272+
build, run_id=run_id, results_dir=results_dir, **kwargs
273+
)
233274

234275

235276
class JavaBenchmarkRunner(BenchmarkRunner):
@@ -310,16 +351,15 @@ def from_rev_or_path(src, root, rev_or_path, maven_conf, **kwargs):
310351
maven_def = JavaMavenDefinition(rev_or_path, maven_conf)
311352
return JavaBenchmarkRunner(maven_def.build(rev_or_path), **kwargs)
312353
else:
313-
# Revisions can references remote via the `/` character, ensure
314-
# that the revision is path friendly
315-
path_rev = rev_or_path.replace("/", "_")
354+
path_rev = _rev_or_path_dirname(src, rev_or_path)
316355
root_rev = os.path.join(root, path_rev)
317-
os.mkdir(root_rev)
356+
os.makedirs(root_rev, exist_ok=True)
318357

319358
clone_dir = os.path.join(root_rev, "arrow")
320-
# Possibly checkout the sources at given revision, no need to
321-
# perform cleanup on cloned repository as root_rev is reclaimed.
322-
src_rev, _ = src.at_revision(rev_or_path, clone_dir)
359+
if os.path.isdir(clone_dir):
360+
src_rev = ArrowSources(clone_dir)
361+
else:
362+
src_rev, _ = src.at_revision(rev_or_path, clone_dir)
323363
maven_def = JavaMavenDefinition(src_rev.java, maven_conf)
324364
build_dir = os.path.join(root_rev, "arrow/java")
325365
return JavaBenchmarkRunner(maven_def.build(build_dir), **kwargs)

dev/archery/archery/cli.py

Lines changed: 61 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,12 @@
1717

1818
from io import StringIO
1919
import click
20+
import datetime
2021
import json
2122
import logging
2223
import os
2324
import pathlib
25+
import platform
2426
import sys
2527

2628
from .benchmark.codec import JsonEncoder
@@ -41,6 +43,27 @@
4143
BOOL = ArrowBool()
4244

4345

46+
def _run_metadata(ctx):
47+
uname = platform.uname()
48+
return {
49+
"time": datetime.datetime.now(datetime.timezone.utc).isoformat(),
50+
"cmd": {
51+
"argv": sys.argv,
52+
"params": ctx.params,
53+
},
54+
"machine_info": {
55+
"platform": platform.platform(),
56+
"system": uname.system,
57+
"release": uname.release,
58+
"version": uname.version,
59+
"machine": platform.machine(),
60+
"processor": platform.processor(),
61+
"architecture": platform.architecture()[0],
62+
"logical_cores": os.cpu_count(),
63+
},
64+
}
65+
66+
4467
@click.group(context_settings={"help_option_names": ["-h", "--help"]})
4568
@click.option("--debug", type=BOOL, is_flag=True, default=False,
4669
envvar='ARCHERY_DEBUG',
@@ -305,6 +328,11 @@ def check_language(ctx, param, value):
305328
click.option("--preserve", type=BOOL, default=False, show_default=True,
306329
is_flag=True,
307330
help="Preserve workspace for investigation."),
331+
click.option("--preserve-dir", metavar="<path>",
332+
type=click.Path(file_okay=False, resolve_path=True),
333+
default=None,
334+
help="Parent directory in which to create the preserved "
335+
"workspace. Implies --preserve."),
308336
click.option("--output", metavar="<output>",
309337
type=click.File("w", encoding="utf8"), default=None,
310338
help="Capture output result into file."),
@@ -347,12 +375,14 @@ def benchmark_filter_options(cmd):
347375
default="WORKSPACE", required=False)
348376
@benchmark_common_options
349377
@click.pass_context
350-
def benchmark_list(ctx, rev_or_path, src, preserve, output, cmake_extras,
351-
java_home, java_options, build_extras, benchmark_extras,
352-
cpp_benchmark_extras, language, **kwargs):
378+
def benchmark_list(ctx, rev_or_path, src, preserve, preserve_dir, output,
379+
cmake_extras, java_home, java_options, build_extras,
380+
benchmark_extras, cpp_benchmark_extras, language, **kwargs):
353381
""" List benchmark suite.
354382
"""
355-
with tmpdir(preserve=preserve) as root:
383+
# A preserve_dir implies preserving the workspace.
384+
preserve = preserve or preserve_dir is not None
385+
with tmpdir(preserve=preserve, preserve_dir=preserve_dir) as root:
356386
logger.debug(f"Running benchmark {rev_or_path}")
357387

358388
if language == "cpp":
@@ -392,10 +422,11 @@ def benchmark_list(ctx, rev_or_path, src, preserve, output, cmake_extras,
392422
"Currently only supported for language=cpp. "
393423
"[default: use runner-specific defaults]"))
394424
@click.pass_context
395-
def benchmark_run(ctx, rev_or_path, src, preserve, output, cmake_extras,
396-
java_home, java_options, build_extras, benchmark_extras,
397-
language, suite_filter, benchmark_filter, repetitions,
398-
repetition_min_time, cpp_benchmark_extras, **kwargs):
425+
def benchmark_run(ctx, rev_or_path, src, preserve, preserve_dir, output,
426+
cmake_extras, java_home, java_options, build_extras,
427+
benchmark_extras, language, suite_filter, benchmark_filter,
428+
repetitions, repetition_min_time, cpp_benchmark_extras,
429+
**kwargs):
399430
""" Run benchmark suite.
400431
401432
This command will run the benchmark suite for a single build. This is
@@ -433,7 +464,9 @@ def benchmark_run(ctx, rev_or_path, src, preserve, output, cmake_extras,
433464
\b
434465
archery benchmark run --output=run.json
435466
"""
436-
with tmpdir(preserve=preserve) as root:
467+
# A preserve_dir implies preserving the workspace.
468+
preserve = preserve or preserve_dir is not None
469+
with tmpdir(preserve=preserve, preserve_dir=preserve_dir) as root:
437470
logger.debug(f"Running benchmark {rev_or_path}")
438471

439472
if language == "cpp":
@@ -465,6 +498,17 @@ def benchmark_run(ctx, rev_or_path, src, preserve, output, cmake_extras,
465498
# when asked to JSON-serialize the results, so produce a JSON
466499
# output even when none is requested.
467500
json_out = json.dumps(runner_base, cls=JsonEncoder)
501+
if runner_base.results_dir is not None:
502+
results_path = os.path.join(runner_base.results_dir,
503+
"benchmark.json")
504+
with open(results_path, "w") as f:
505+
f.write(json_out)
506+
# Store some run metadata to make it hard to confuse runs, for instance
507+
# remembering what was the SIMD level set on this run.
508+
metadata_path = os.path.join(runner_base.results_dir,
509+
"metadata.json")
510+
with open(metadata_path, "w") as f:
511+
json.dump(_run_metadata(ctx), f, indent=2, default=str)
468512
if output is not None:
469513
output.write(json_out)
470514

@@ -486,11 +530,11 @@ def benchmark_run(ctx, rev_or_path, src, preserve, output, cmake_extras,
486530
@click.argument("baseline", metavar="[<baseline>]]", default="origin/HEAD",
487531
required=False)
488532
@click.pass_context
489-
def benchmark_diff(ctx, src, preserve, output, language, cmake_extras,
490-
suite_filter, benchmark_filter, repetitions, no_counters,
491-
java_home, java_options, build_extras, benchmark_extras,
492-
cpp_benchmark_extras, threshold, contender, baseline,
493-
**kwargs):
533+
def benchmark_diff(ctx, src, preserve, preserve_dir, output, language,
534+
cmake_extras, suite_filter, benchmark_filter, repetitions,
535+
no_counters, java_home, java_options, build_extras,
536+
benchmark_extras, cpp_benchmark_extras, threshold,
537+
contender, baseline, **kwargs):
494538
"""Compare (diff) benchmark runs.
495539
496540
This command acts like git-diff but for benchmark results.
@@ -564,7 +608,9 @@ def benchmark_diff(ctx, src, preserve, output, language, cmake_extras,
564608
# This should not recompute the benchmark from run.json
565609
archery --quiet benchmark diff WORKSPACE run.json > result.json
566610
"""
567-
with tmpdir(preserve=preserve) as root:
611+
# A preserve_dir implies preserving the workspace.
612+
preserve = preserve or preserve_dir is not None
613+
with tmpdir(preserve=preserve, preserve_dir=preserve_dir) as root:
568614
logger.debug(f"Comparing {contender} (contender) with {baseline} (baseline)")
569615

570616
if language == "cpp":

dev/archery/archery/utils/tmpdir.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,17 @@
1515
# specific language governing permissions and limitations
1616
# under the License.
1717

18+
import os
1819
from contextlib import contextmanager
1920
from tempfile import mkdtemp, TemporaryDirectory
2021

2122

2223
@contextmanager
23-
def tmpdir(preserve=False, prefix="arrow-archery-"):
24-
if preserve:
24+
def tmpdir(preserve=False, prefix="arrow-archery-", preserve_dir=None):
25+
if preserve and preserve_dir is not None:
26+
os.makedirs(preserve_dir, exist_ok=True)
27+
yield preserve_dir
28+
elif preserve:
2529
yield mkdtemp(prefix=prefix)
2630
else:
2731
with TemporaryDirectory(prefix=prefix) as tmp:

0 commit comments

Comments
 (0)