Benchmarks: Micro benchmark - add nvbench based kernel-launch, sleep-kernel & auto-throughput - #750
Open
WenqingLan1 wants to merge 59 commits into
Open
Benchmarks: Micro benchmark - add nvbench based kernel-launch, sleep-kernel & auto-throughput#750WenqingLan1 wants to merge 59 commits into
WenqingLan1 wants to merge 59 commits into
Conversation
WenqingLan1
added
benchmarks
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #750 +/- ##
==========================================
+ Coverage 86.02% 86.29% +0.27%
==========================================
Files 103 107 +4
Lines 7950 8225 +275
==========================================
+ Hits 6839 7098 +259
- Misses 1111 1127 +16
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| return val | ||
|
|
||
|
|
||
| _NVBENCH_INT_VALUES_PATTERN = re.compile( |
Contributor
There was a problem hiding this comment.
To pass the lint check, pls use 1 line here.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 28 changed files in this pull request and generated no new comments.
Suppressed comments (3)
superbench/benchmarks/micro_benchmarks/nvbench_auto_throughput.py:41
--block_sizeusesparse_nvbench_int_values, which also accepts NVBench range formats (e.g.[128:1024]/[128:1024:128]), but the help text only documents single-value and list formats. Please either document the supported range formats or restrict parsing for this argument.
self._parser.add_argument(
'--block_size',
type=parse_nvbench_int_values,
default='[128,256,512,1024]',
help='Block size (threads per block). Supports: "256" (single), "[128,256,512,1024]" (list).',
superbench/benchmarks/micro_benchmarks/nvbench_base.py:23
parse_time_to_usallows multiple dots in the numeric portion via([\d.]+), so inputs like"1..2 us"match the regex but then fail with a genericfloat()conversion error. Tightening the regex and normalizing the error makes invalid inputs fail consistently withInvalid time string: ....
raw = raw.strip()
m = re.match(r'^([\d.]+)\s*([mun]?s)?$', raw)
if not m:
raise ValueError(f'Invalid time string: {raw!r}')
val, unit = float(m.group(1)), (m.group(2) or 'us')
superbench/benchmarks/micro_benchmarks/nvbench_sleep_kernel.py:52
- The
_process_raw_resultdocstring saysself._result.add_raw_data()needs to be called, but raw JSON is already recorded byNvbenchBase._load_result_json(). This is misleading for anyone implementing new NVBench benchmarks based on this example.
"""Function to parse raw results and save the summarized results.
self._result.add_raw_data() and self._result.add_result() need to be called to save the results.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 29 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
superbench/benchmarks/micro_benchmarks/nvbench_base.py:44
parse_nvbench_int_valuescurrently accepts whitespace inside bracketed list/range forms (e.g. "[0, 1]") and returns the value unchanged. That can later produce invalid CLI tokens (especially for--devices) if spaces are present, and also makes it easier to accidentally generate commands that won’t parse as intended. Consider normalizing by stripping all whitespace after validation and returning the normalized value.
def parse_nvbench_int_values(value):
"""Validate an NVBench integer value specification."""
# Accepted formats: '0', '[0,1,2]', '[0:4]', and '[0:4:2]' (range with step).
if not _NVBENCH_INT_VALUES_PATTERN.fullmatch(value):
raise ValueError(
'Invalid NVBench integer values. Use a single value like "0", '
'a list like "[0,1,2]", or a range like "[0:4]" or "[0:4:2]".'
)
return value
superbench/benchmarks/micro_benchmarks/nvbench_base.py:184
--devicesvalues like "[0,1,2]" / "[0:4]" contain shell glob metacharacters ([and]). Since commands are executed withshell=True(seerun_command), passing these unquoted can trigger glob expansion based on the working directory contents, producing unexpected arguments. Quoting/escaping the--devicesvalue when building the command would make behavior deterministic.
def _add_device_args(self, parts):
"""Add device configuration arguments to command parts."""
if hasattr(self._args, 'devices') and self._args.devices is not None:
if self._args.devices == 'all':
parts.extend(['--devices', 'all'])
else:
parts.extend(['--devices', self._args.devices])
Comment on lines
+49
to
+53
| """Function to parse raw results and save the summarized results. | ||
|
|
||
| self._result.add_raw_data() and self._result.add_result() need to be called to save the results. | ||
|
|
||
| Args: |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 28 changed files in this pull request and generated no new comments.
Suppressed comments (2)
superbench/benchmarks/micro_benchmarks/nvbench/auto_throughput.cu:63
Strideaxis is currently defined asnvbench::range(1, 4, 3), which only generates strides 1 and 4. This conflicts with the Python wrapper / docs that accept ranges like[1:4](implying 1,2,3,4) and examples like[1,2,4,8], and can lead to missing/empty states at runtime depending on what the user requests.
Consider defining the stride axis explicitly to match the supported/user-documented values (e.g., {1, 2, 4, 8}), or adjust the wrapper/docs to only allow the strides produced by this benchmark.
.add_int64_axis("Stride", nvbench::range(1, 4, 3))
.github/workflows/codeql-analysis.yml:59
- This workflow installs CMake 3.20.0, but this PR introduces nvbench build logic that explicitly gates on CMake >= 3.30.4 (e.g.,
superbench/benchmarks/micro_benchmarks/nvbench/CMakeLists.txt). With 3.20.0, the nvbench benchmarks will be skipped, so the new CUDA sources likely won’t be built/analyzed by the CodeQL C++ job.
Also, lukka/get-cmake@latest is not reproducible; please pin the action to a specific release tag or commit SHA.
- name: Setup CMake
uses: lukka/get-cmake@latest
with:
cmakeVersion: '3.20.0'
Hongtao Zhang (polarG)
approved these changes
Aug 25, 2026
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
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.
This pull request adds support for NVBench-based GPU micro-benchmarks to SuperBench.
nvbench-sleep-kernelnvbench-kernel-launchnvbench-auto-throughputExample config: