Skip to content

Simplify and speed up param packer - #2904

Merged
seberg merged 3 commits into
NVIDIA:mainfrom
seberg:simplify-param-packer
Sep 18, 2026
Merged

seberg merged 3 commits into
NVIDIA:mainfrom
seberg:simplify-param-packer

Conversation

@seberg

@seberg seberg commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

This simplifies the param packer and stays in Cython. The original motivation was a small regression found by the benchmarks. But this also speeds up the same cases by a large factor. For many arguments up to 10x.
That many arguments are not too interesting, but also 16 have a 16% speedup at least.

Some further optimizations are certainly plausible and may become more relevant as kernel launch time decreases. I limited it to only a few things:

  • Move to Cython (for simplificty)
  • Reorder/avoid many isinstance() checks
  • Also fixes potentially unaligned memory access. Although, this may just not matter for any platform.

There are a bunch of smaller things I could think of (but didn't do to keep this a more mechanical "move" and I am not sure how hot these are):

  • Remove isinstance() checks mostly for exact type checks
  • use getattr(type(value), 'getPtr', None) on the type (avoid binding and stricter contract).
  • Make the helper a C++ class (may save close to 5% on empty kernel, but added complexity).
  • ...

Benchmark

This really changes all except the last two lines (the last two pass an existing buffer or )

Benchmark main (µs/launch) This PR (µs/launch) Speedup
bench_launch_512_doubles 64.78 6.44 10.1×
bench_launch_512_ints 46.05 5.77 8.0×
bench_launch_512_bools 43.54 6.60 6.6×
bench_launch_512_args (all-None types) 26.67 19.16 1.39×
bench_launch_16_args 2.68 2.31 1.16×
bench_launch_512_args_pre_packed 3.17 3.14 1.01×
bench_launch_empty_kernel 1.64 1.62 1.01×
Details

AMD Ryzen Threadripper PRO 7975WX / NVIDIA RTX PRO 6000 Blackwell (sm_120),
driver 580.173.02, CUDA 13.0, CPython 3.14.7 (GIL build), Cython 3.2.9,
g++ -O3. Both sides built through an identical pipeline from a detached-HEAD
worktree and the PR branch. 1500 launches per repetition, min of 5 repetitions,
7 rounds interleaved, median of per-round minima; process pinned to one core on
an otherwise idle machine.

The last two rows are controls: bench_launch_empty_kernel passes
kernelParams=0 and bench_launch_512_args_pre_packed supplies an
already-packed buffer, so neither reaches the packing path and both are
expected to be flat.

This simplifies the param packer and stays in Cython.
The original motivation was a (honestly) minor speed regression, but
this also speeds up the same cases by a large factor for many
arguments (up to 10x).
Many arguments are not too interesting, but also 16 have a 16%
speedup at least.

Some other optimizations are certainly plausible and may become
more relevant as kernel launch time decreases.
@seberg seberg self-assigned this Sep 17, 2026
@seberg seberg added enhancement Any code-related improvements P1 Medium priority - Should do cuda.bindings Everything related to the cuda.bindings module labels Sep 17, 2026
@seberg seberg added this to the cuda.bindings next milestone Sep 17, 2026
@github-actions

This comment has been minimized.

@mdboom mdboom left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. My agent found no additional issues.

This is the kind of thing we would normally port to 12.9.x as well, if these patches apply cleanly there. The generated changes will automatically make it there eventually, but they seem independent enough that if the rest weren't ported it would be ok.

@seberg

seberg commented Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

but they seem independent enough that if the rest weren't ported it would be ok.

Hmmm, I suspect the generated changes would break without the rest unfortunately. But this should apply cleanly (enough), just means need to follow-up on that.

@seberg

seberg commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

Created a manual backport in gh-2911, hopefully that is enough to not worry about that!

@seberg
seberg merged commit 4fd8f50 into NVIDIA:main Sep 18, 2026
125 of 130 checks passed
@seberg
seberg deleted the simplify-param-packer branch September 18, 2026 08:24
@github-actions

Copy link
Copy Markdown
Contributor
Doc Preview CI
Preview removed because the pull request was closed or merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cuda.bindings Everything related to the cuda.bindings module enhancement Any code-related improvements P1 Medium priority - Should do

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants