Benchmarks: Micro benchmark - add nvbench based kernel-launch, sleep-kernel & auto-throughput - #750
Open
WenqingLan1 wants to merge 63 commits into
Open
Benchmarks: Micro benchmark - add nvbench based kernel-launch, sleep-kernel & auto-throughput#750WenqingLan1 wants to merge 63 commits into
WenqingLan1 wants to merge 63 commits into
Conversation
WenqingLan1
added
benchmarks
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #750 +/- ##
==========================================
+ Coverage 86.02% 86.30% +0.27%
==========================================
Files 103 107 +4
Lines 7950 8227 +277
==========================================
+ Hits 6839 7100 +261
- 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:
|
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])
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
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)
Previously missed (2) — in code that hasn't changed since the last review.
superbench/benchmarks/micro_benchmarks/nvbench_base.py:268
- NvbenchBase._preprocess() calls _finalize_commands(), which can raise OSError (e.g. an unwritable --output_dir). That exception bubbles up to Benchmark.run() and becomes a RUNTIME_EXCEPTION_ERROR instead of an INVALID_ARGUMENT, making misconfiguration harder to diagnose and inconsistent with other argparse validation failures.
if not super()._preprocess():
return False
parts = self._extend_command(self._build_base_command())
self._commands = self._finalize_commands([' '.join(parts)])
return True
superbench/benchmarks/micro_benchmarks/nvbench_base.py:44
- parse_nvbench_int_values() currently accepts whitespace inside bracketed list/range specs (e.g. "[0, 1]"), but returns the original string. This can later produce broken shell commands (notably for --devices, which is appended unquoted) because spaces will be split into separate argv tokens by the shell.
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
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 1 comment.
Suppressed comments (1)
.github/workflows/codeql-analysis.yml:59
- The new nvbench CMake gating requires CMake >= 3.30.4, but the CodeQL C++ workflow installs CMake 3.20.0. As a result, the nvbench micro-benchmark CMake project will always short-circuit and the newly added CUDA code won’t be built/analyzed in this workflow. Bump the configured CMake version (and consider pinning
lukka/get-cmaketo a tag/SHA for reproducibility).
- name: Setup CMake
uses: lukka/get-cmake@latest
with:
cmakeVersion: '3.20.0'
…erbenchmark into feat/third_party/nvbench
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 (4)
Previously missed (3) — in code that hasn't changed since the last review.
superbench/benchmarks/micro_benchmarks/nvbench/CMakeLists.txt:49
- nvbench benchmark executables don’t set CUDA_ARCHITECTURES, unlike other CUDA micro-benchmarks. This can result in binaries being built only for an unintended/default architecture set, causing runtime failures on GPUs whose SMs aren’t included. Set CUDA_ARCHITECTURES for each nvbench_* target (e.g., to NVCC_ARCHS_SUPPORTED from cuda_common.cmake).
add_executable(${target} ${src})
target_compile_features(${target} PUBLIC cuda_std_17)
target_link_libraries(${target}
PRIVATE nvbench::nvbench nvbench::main
)
install(TARGETS ${target} RUNTIME DESTINATION bin)
superbench/benchmarks/micro_benchmarks/nvbench_auto_throughput.py:42
- --block_size currently accepts "0" (or ranges/steps containing 0) because it uses parse_nvbench_int_values. That can lead to a host-side division-by-zero when computing blocks_in_grid and/or an invalid kernel launch configuration. Add validation to ensure block_size values are in a safe range (at minimum >0; typical CUDA limit is <=1024 threads/block).
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_auto_throughput.py:97
- auto-throughput’s CUDA code notes CUPTI collection calls are no-ops if NVBench wasn’t built with CUPTI, which means the corresponding nv/cupti/* summary tags may be absent. Currently the parser treats missing CUPTI tags as a hard failure via _summary_value(), so the whole benchmark can fail even though timing metrics are available. Consider making CUPTI metrics optional by skipping missing tags instead of failing parsing.
# CUPTI metrics (fraction -> percentage)
for metric, tag in cupti_tags.items():
self._result.add_result(f'{prefix}_{metric}', self._summary_value(summaries, tag) * 100)
.github/workflows/codeql-analysis.yml:59
- This workflow pins CMake to 3.20.0 and uses lukka/get-cmake@latest. CMake 3.20.0 guarantees the new nvbench micro-benchmarks will be skipped by superbench/benchmarks/micro_benchmarks/nvbench/CMakeLists.txt (which requires >= 3.30.4), so the added CUDA/C++ code won’t be built or analyzed by CodeQL. Also, using
@latestmakes the workflow non-reproducible; please pin the action to a specific version/commit and decide whether nvbench code should be included in CodeQL builds (by bumping CMake) or explicitly excluded/reverted here.
DEBIAN_FRONTEND=noninteractive apt-get install -y ffmpeg libavcodec-dev libavformat-dev libavutil-dev libswresample-dev sudo build-essential
- name: Setup CMake
uses: lukka/get-cmake@latest
with:
cmakeVersion: '3.20.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
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: