test(benchmarks): add --reuse-data flag to reuse listing benchmark fixtures - #1083
raj-prince wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a --reuse-data option to reuse static benchmark data across runs, skipping file and folder creation if the data already exists. The feedback suggests improving the robustness of the reuse check by using a sentinel file (e.g., .setup_complete) instead of checking only the first file, which could lead to reusing incomplete datasets from interrupted runs. Additionally, it is recommended to simplify the argument check in run.py to directly access args.reuse_data.
|
|
||
| params.files = len(file_paths) | ||
|
|
||
| reuse_data = bool(reuse and file_paths and gcs.exists(file_paths[0])) |
There was a problem hiding this comment.
Checking only the first file (gcs.exists(file_paths[0])) to determine if the dataset can be reused is prone to partial/incomplete runs. If a previous benchmark setup was interrupted or failed midway, some files (including the first one) might exist, but the dataset would be incomplete. This would lead to subsequent runs silently reusing invalid/incomplete data, producing incorrect benchmark results.
A more robust approach is to use a sentinel file (e.g., .setup_complete) that is only written after the entire setup process (folder and file creation) successfully completes. We can then check for the existence of this sentinel file to safely reuse the data.
sentinel_path = f"{prefix}/.setup_complete"
reuse_data = bool(reuse and gcs.exists(sentinel_path))| duration_ms = (time.perf_counter() - start_time) * 1000 | ||
| logging.info( | ||
| f"Benchmark '{params.name}' setup created {len(file_paths)} files in {duration_ms:.2f} ms." | ||
| ) |
There was a problem hiding this comment.
To support the sentinel-based reuse check, write the .setup_complete sentinel file once the file preparation successfully completes.
| duration_ms = (time.perf_counter() - start_time) * 1000 | |
| logging.info( | |
| f"Benchmark '{params.name}' setup created {len(file_paths)} files in {duration_ms:.2f} ms." | |
| ) | |
| duration_ms = (time.perf_counter() - start_time) * 1000 | |
| logging.info( | |
| f"Benchmark '{params.name}' setup created {len(file_paths)} files in {duration_ms:.2f} ms." | |
| ) | |
| gcs.pipe({sentinel_path: b""}) |
| if args.config: | ||
| os.environ["GCSFS_BENCHMARK_FILTER"] = ",".join(args.config) | ||
|
|
||
| if getattr(args, "reuse_data", False): |
There was a problem hiding this comment.
Since --reuse-data is defined as an argument in main(), args.reuse_data is guaranteed to exist on the args namespace. We can simplify this check to if args.reuse_data: for consistency with how other arguments (like args.config) are accessed.
| if getattr(args, "reuse_data", False): | |
| if args.reuse_data: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1083 +/- ##
=======================================
Coverage 90.25% 90.25%
=======================================
Files 16 16
Lines 3755 3755
=======================================
Hits 3389 3389
Misses 366 366 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Description
Setting up and tearing down listing microbenchmarks often takes several minutes because they create tens to hundreds of thousands of files and folders in GCS. When iterating on optimizations or profiling listing code, re-creating and deleting the same directory structure on every single run creates a significant bottleneck.
This PR introduces a
--reuse-dataflag torun.pyto allow listing benchmarks to reuse pre-existing test data across runs:gs://<bucket>/benchmark-static/<test_name>instead of generating a randomized UUID prefix.This change is intentionally scoped solely to listing benchmarks (
_benchmark_listing_fixture_helper), where fixture creation overhead is highest.Impact / Benchmark Iteration Time
For a
list_flatrun (65k and 131k files):--reuse-data: ~14s (>10x faster iteration loop)How to Test
Unit tests pass: