[Feature] Extract reusable event-based benchmarking helper#2
Draft
jhinpan wants to merge 1 commit into
Draft
Conversation
Signed-off-by: Jin Pan <jpan236@wisc.edu>
Reviewer's GuideExtract the GPU event-based benchmarking helper into a reusable profiling module while preserving autotune compatibility, adding validation and setup-hook support, and documenting the new API with unit tests. Sequence diagram for GPU event-based benchmarking with setup hooksequenceDiagram
actor BenchmarkCaller
participant profiling as flydsl.profiling.do_bench
participant torch_cuda as torch.cuda
BenchmarkCaller->>profiling: do_bench(fn, warmup, rep, quantiles, setup)
profiling->>profiling: validate warmup, rep, quantiles
profiling->>torch_cuda: _get_torch_cuda()
torch_cuda-->>profiling: cuda_namespace
loop warmup iterations
alt setup is not None
profiling->>BenchmarkCaller: setup()
end
profiling->>BenchmarkCaller: fn()
end
profiling->>torch_cuda: synchronize()
loop rep iterations
alt setup is not None
profiling->>BenchmarkCaller: setup()
end
profiling->>torch_cuda: Event(enable_timing=True) as start
profiling->>torch_cuda: Event(enable_timing=True) as end
profiling->>torch_cuda: start.record()
profiling->>BenchmarkCaller: fn()
profiling->>torch_cuda: end.record()
profiling->>torch_cuda: synchronize()
profiling->>torch_cuda: start.elapsed_time(end)
end
profiling->>profiling: sort(times)
alt quantiles is not None and non-empty
profiling-->>BenchmarkCaller: selected quantile samples
else
profiling-->>BenchmarkCaller: upper-middle median sample
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the GPU benchmarking utility do_bench by moving it from autotune.py to a new shared module profiling.py, accompanied by updated documentation and unit tests. The review feedback correctly identifies an issue where passing quantiles as an iterator or generator would exhaust it during validation, leading to an empty result. It is recommended to convert quantiles to a list first and add a unit test to cover this scenario.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+53
to
+54
| if quantiles is not None and any(not 0.0 <= q <= 1.0 for q in quantiles): | ||
| raise ValueError("quantiles must be between 0 and 1") |
There was a problem hiding this comment.
If quantiles is passed as an iterator or generator (e.g., a generator expression or a map object), checking it with any(...) will completely exhaust the iterator. Consequently, the subsequent list comprehension on line 78 will iterate over an empty generator and return an empty list [] instead of the expected quantiles.
To prevent this, convert quantiles to a list first if it is not None. This ensures the input is safely consumed and can be iterated over multiple times.
Suggested change
| if quantiles is not None and any(not 0.0 <= q <= 1.0 for q in quantiles): | |
| raise ValueError("quantiles must be between 0 and 1") | |
| if quantiles is not None: | |
| quantiles = list(quantiles) | |
| if any(not 0.0 <= q <= 1.0 for q in quantiles): | |
| raise ValueError("quantiles must be between 0 and 1") |
Comment on lines
+76
to
+83
| def test_do_bench_quantiles(monkeypatch): | ||
| device = _install_fake(monkeypatch) | ||
| durations = iter([3.0, 1.0, 5.0, 2.0, 4.0]) | ||
|
|
||
| def fn(): | ||
| device.clock += next(durations) | ||
|
|
||
| assert profiling.do_bench(fn, warmup=0, rep=5, quantiles=[0.0, 0.5, 0.9, 1.0]) == [1.0, 3.0, 5.0, 5.0] |
There was a problem hiding this comment.
Add a test case to verify that do_bench correctly handles quantiles when passed as a generator, ensuring it does not get prematurely exhausted by the validation checks.
Suggested change
| def test_do_bench_quantiles(monkeypatch): | |
| device = _install_fake(monkeypatch) | |
| durations = iter([3.0, 1.0, 5.0, 2.0, 4.0]) | |
| def fn(): | |
| device.clock += next(durations) | |
| assert profiling.do_bench(fn, warmup=0, rep=5, quantiles=[0.0, 0.5, 0.9, 1.0]) == [1.0, 3.0, 5.0, 5.0] | |
| def test_do_bench_quantiles(monkeypatch): | |
| device = _install_fake(monkeypatch) | |
| durations = iter([3.0, 1.0, 5.0, 2.0, 4.0]) | |
| def fn(): | |
| device.clock += next(durations) | |
| # Test with a list | |
| assert profiling.do_bench(fn, warmup=0, rep=5, quantiles=[0.0, 0.5, 0.9, 1.0]) == [1.0, 3.0, 5.0, 5.0] | |
| # Test with a generator to ensure it doesn't get exhausted prematurely | |
| durations = iter([3.0, 1.0, 5.0, 2.0, 4.0]) | |
| quantiles_gen = (q for q in [0.0, 0.5, 0.9, 1.0]) | |
| assert profiling.do_bench(fn, warmup=0, rep=5, quantiles=quantiles_gen) == [1.0, 3.0, 5.0, 5.0] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extract the existing GPU event benchmark helper from
flydsl.autotuneinto the reusableflydsl.profilingmodule.This is intentionally a narrow, non-intrusive first slice of ROCm/FlyDSL#304. It does not move the broader
tests/test_common.pyutilities or change any kernel, compiler, or backend implementation.Motivation
FlyDSL already has a small event-based
do_benchimplementation, but it is nested inside the autotune module. Moving it to a shared package-level module lets other profiling and benchmarking callers reuse the same behavior and gives the autotune path in ROCm/FlyDSL#770 a clearer dependency boundary.The module does not import ROCDL, NVVM, or FlyDSL compiler bindings. Its current runtime implementation lazily uses PyTorch's
torch.cudaevent interface, which PyTorch also exposes on HIP builds.This PR does not implement or claim validation of a FlyDSL NVVM backend. Runtime/backend placement for the future dual-backend direction remains related to ROCm/FlyDSL#813; the focused namespace question is recorded on ROCm/FlyDSL#304.
Changes
flydsl.profiling.do_bench.setuphook introduced in ROCm/FlyDSL#785, running it before each warmup and timed iteration but before the start event so restore/reset work is excluded from kernel latency.flydsl.autotune.do_benchas a compatibility alias.Compatibility is preserved for existing autotune callers: the import path, valid call signature, defaults, and result shape remain unchanged.
Performance
Not applicable. This is a direct extraction of the existing timer and does not change generated kernels or compiler behavior.
A live MI350X/HIP smoke test returned a valid event-based median timing.
Testing
GPU-free profiling and existing autotune unit tests:
PYTHONPATH="$PWD/python${PYTHONPATH:+:$PYTHONPATH}" \ python3 -m pytest tests/unit/test_profiling.py tests/unit/test_autotune.py -qResult:
31 passed.Repository Python style gate (
bash scripts/check_python_style.sh --include-local).Targeted Black and Ruff checks for all changed Python files.
MI350X/HIP event-timing smoke test.
CUDA/NVVM execution was not tested and is not claimed by this PR.
Dependencies
Breaking Changes
None for valid existing calls.
flydsl.autotune.do_benchremains available as an alias to the shared implementation.Upstream references: #304, #770, #813.
Summary by Sourcery
Extract the existing GPU event-based benchmarking helper into a shared profiling module while preserving autotune behavior and public API semantics.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: