Repository navigation
fix(cuda-macros): load kernels through a vector so cleanup stays linear - #1588
sachinsharma3191 wants to merge 1 commit into
Conversation
|
Force-pushed a corrected version of this, and a correction to what the The first claim was too strongThe original push said the cleanup becomes linear. It does not, in the shape I
Exponent 1.2, then 1.5, then 2.8. Better than before, still the wrong shape. What it does nowLoad into a vector, convert that to a fixed-size array, then bind every let mut __cuda_oxide_functions: Vec<CudaFunction> = Vec::with_capacity(2usize);
__cuda_oxide_functions.push(module.load_function(/* ... */)?);
__cuda_oxide_functions.push(module.load_function(/* ... */)?);
let __cuda_oxide_loaded: [CudaFunction; 2usize] = __cuda_oxide_functions
.try_into()
.expect("every declared kernel was loaded above");
let [__cuda_oxide_loaded__first_function, __cuda_oxide_loaded__second_function] =
__cuda_oxide_loaded;
Ok(LoadedModule { /* fields take the locals */ })The conversion is the only fallible step and is unreachable by construction A Measurements
Wall clock on a laptop is noisy — the iterator's 200-kernel cell measured
Per doubling: 3.78x then 3.90x for the original (4x is quadratic), 1.81x then Still not verified end to endI have no CUDA toolkit on this machine, so I could not build a real @navneet83 — if you still have the generator, a measurement of the real |
e488b4b to
916d909
Compare
A module's loaders built `LoadedModule` in one struct literal whose field
initializers each ended in `?`. Every one of those sits inside a half-built
struct: the fields evaluated before it are live and each owns an `Arc`
(`CudaFunction` holds one), so the i-th early return has to drop i values.
Summed over a module's kernels that is O(n^2) cleanup from O(n) expansion,
and host build time followed -- 200 kernels in one `#[cuda_module]` took 98s
against 8s for 100, with nearly all of it inside LLVM.
Load into a vector, convert that to a fixed-size array, then bind every
function with one irrefutable pattern. The conversion is the only fallible
step, and nothing after it can unwind, so the moves that fill the struct emit
no cleanup edges at all.
A `cfg`'d kernel leaves the count unknown until the compiler evaluates it,
and a pattern cannot carry `#[cfg]` per element, so such a module keeps an
iterator: one push and one take per kernel, each under its own `cfg`, which
statements can carry. That tail is still quadratic on the panic path, but
with a far smaller constant than the original.
All three loaders share the helper: both `from_module` variants and the
nested namespace's `from_parent`, which had the identical shape.
Measured with a generated crate that mimics the expansion, compiled with
rustc -O (the dash marks sizes the old shape is too slow to measure here):
kernels field initializers iterator array
50 0.92s 0.35s 0.40s
100 7.71s 0.55s 0.51s
200 93.58s 2.51s 0.84s
400 - 3.46s 1.81s
800 - 23.47s 5.36s
Generated LLVM IR lines, which confirm the shape independently of the
machine:
kernels field initializers iterator array
50 26,292 4,528 3,350
100 99,417 8,195 6,067
200 388,167 14,895 10,867
Per doubling the old form grows 3.78x then 3.90x, the array 1.81x then
1.79x. At the 200 kernels the report measured, that is 93.58s -> 0.84s.
Closes NVIDIA#1501.
Signed-off-by: sachin <[email protected]>
916d909 to
90e2ee2
Compare
Closes #1501.
Why it is superquadratic
A module's loaders build
LoadedModulein one struct literal whose fieldinitializers each end in
?:Every
?there sits inside a half-built struct. The fields evaluated beforeit are live, and each owns an
Arc—CudaFunctionholds one to keep themodule loaded — so the i-th early return has to drop i values. Over a
module's kernels that is 1+2+...+n drop calls: O(n²) cleanup generated from
O(n) expansion, which matches the report's observation that expansion stayed
linear at about 106 lines per kernel while LLVM time did not.
The change
Load into a vector first, then take the functions back out in order:
Exactly one droppable value is live at each
?, so the cleanup is linear.A push per kernel, rather than a loop over a name table, is what preserves
per-kernel
cfg: statements take attributes, const array elements do not.Taking the functions back out is infallible by construction — both halves are
emitted from one iterator, in one order, under the same
cfgs — so theexpectis unreachable rather than a new failure mode.All three loaders share the new helper: both
from_modulevariants(contracted and uncontracted) and the nested namespace's
from_parent, whichhad the identical shape. Fixing only
from_modulewould have left nestedmodules quadratic.
No API change: field names, types, load order and error type are unchanged.
Measurements
I could not build the reporter's crate (no CUDA toolkit on this machine), so I
reproduced the shape instead: a generated crate with a droppable
CudaFunctionequivalent, N fields, and the two loader shapes, compiled withrustc -Oat the kernel counts from the report.Between 100 and 200 kernels the old shape grows at an exponent near 4 and the
new one near 1.2; at 200 kernels it is 75x faster. The 95s for the old shape
lands within 3% of the 98s reported for a real 200-kernel crate, which is
what makes me reasonably confident the loader is the dominant term rather
than one contributor among several.
@navneet83, if you still have the generator, a re-measure of the real
200-kernel crate against this branch would confirm it end to end — in
particular whether the
LLVM_thinltoshare you measured drops with it. The Nlaunch methods are untouched here, so if a superlinear term remains it should
show up in that split.
Testing
kernels_load_into_a_vector_instead_of_the_struct_literalexpands a module with two kernels and a nested namespace, and asserts every
load is a push on one vector, that no kernel loads directly into a struct
field, that both loaders use a vector, and that each kernel takes its
function back out. The second assertion is the regression: it fails on the
previous shape.
cuda-macros' unit tests pass.cargo clippy -p cuda-macros --lib -- -D warningsandcargo oxide fmt --checkare clean.crates/cuda-macros/tests/, andanything else needing a CUDA toolkit.
cuda-macros' dev-dependencies reachcuda-bindings, whose build script needscuda.h, so those run in CI here.