Uh oh!
There was an error while loading. Please reload this page.
lib-cuda: record the measured cost of the 32-bit index axis on this backend - #151
Merged
Conversation
The knob's cost was previously quoted from a host-compiler measurement (fastfields-lib#94) and an assumption that CUDA would behave the same way. The `build-cuda` matrix added in the previous commit builds both positions in one run, so the CUDA figures are now measured rather than extrapolated: peak nvcc RSS and wall time per module, side by side, from a single CI run on one runner class. They are worse than the host-compiler extrapolation suggested, not better -- reg_flow drops 56.1% rather than the 44.6% measured with clang, and the whole compile drops 49.3% of nvcc CPU (~38 min -> ~19 min wall). Also notes, because the table below claims the heavy modules reproduce to better than 0.1%: reg_flow does (12.97 vs the recorded 12.98 GiB), reg_field does not (8.09 vs a recorded 8.93). Recording that rather than quietly overwriting the older figure. Comment-only; no build behaviour changes.
Uh oh!
There was an error while loading. Please reload this page.
balbasty pushed a commit
that referenced
this pull request
Aug 20, 2026
Brings in #143 (per-backend FF_INDEX32), #145 (#pragma once), #146 (<fastfields/...> includes) and #151 (the measured index-axis table). One conflict, in src/lib-cuda/reg_flow.cpp: this branch splits the file and moves the dtype x offset dispatch macros into reg_flow_slice.inl, while #143 rewrote the narrow arm of those same macros from int32_t to off32_t and #146 rewrote the include lines. Resolved by taking this branch's structure with both of main's changes applied in their new home: * all 20 narrow dispatch arms in reg_flow_slice.inl now name off32_t, which is exactly the count main's reg_flow.cpp carries; * the new files already spelled public includes <fastfields/...> and kept the same-directory sibling quoted, so #146 needed nothing; * reg_flow_slice.h switched from an #ifndef guard to #pragma once -- #145 landed the opposite way round from its original proposal, and the new files followed the proposal rather than the merge. tools/normalise-header-guards.py --check, tools/normalise-include-delimiters.py --check and tools/rename-macros.py --check are all clean on the result. Re-verified after the merge: * tools/test-baseline.sh --legs default,lib -> row-for-row identical to tools/test-baseline.expected across all 15 rows. 59,886 checks / 13 suites for the default leg, plus the 2 hub suites, 0 failures. * every one of the 30 slice functions is still defined exactly once across the twelve slice TUs (preprocessor census: 13 occurrences each -- twelve declarations from the shared header plus one definition). * the front TU still instantiates nothing: 205,400 kB, 1.8 s. * the heaviest slice still compiles: 2,037,652 kB, matching the 2,039,660 kB build-cuda reported for it before the rebase (0.1%).
balbasty added a commit
that referenced
this pull request
Aug 20, 2026
#151 recorded what FF_INDEX32 costs on the unsplit modules. This is the same question asked of a slice, because the interesting case is whether splitting makes the axis cheap enough that the knob stops mattering. It does not. Same TU, index axis toggled: reg_flow_matvec_3d FF_INDEX32=1 1.94 GiB 277.1 s 17.0 MB FF_INDEX32=0 1.05 GiB 174.4 s 10.0 MB -45.8% peak, which is the same proportion #94 measured on the unsplit file with the host compiler (-44.6%). The split and the knob multiply rather than overlap: 12.98 -> 1.94 GiB from one, 1.94 -> 1.05 GiB from the other.
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 freeto 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.
Comment-only follow-up to #143, which merged before these numbers were in hand.
No build behaviour changes.
#143 landed
INDEXFLAGS/FF_INDEX32and madebuild-cudaa two-leg matrixover the two positions. Its own text still quoted the axis's cost from the
host-compiler measurement in #94 plus an assumption that CUDA would behave
similarly. The matrix's first run has now produced the real CUDA figures, so
this records them above
MODULESinsrc/lib-cuda/Makefile— both positions,one CI run, one runner class,
nvcc -O1 -j2with the shipping bound/splinepolicy.
Peak nvcc RSS (GiB) / wall per module:
FF_INDEX32=1FF_INDEX32=0reg_flowreg_fieldreg_field_rlsresizereg_flow_rlspushpull_backwardpushpullrestrictdistancesplincposdefSource: run
32363152580,
jobs
build-cuda (index32)andbuild-cuda (index64), both green.Two things this makes visible, and the reason for recording rather than
quietly updating:
reg_flowsheds 56.1% of peak RSS here, against the 44.6% clang showed.says the heavy modules reproduce to better than 0.1%.
reg_flowdoes — 12.97against the recorded 12.98 GiB.
reg_fieldcame in at 8.09 against arecorded 8.93, ~9% low. Both figures are now in the file with that noted,
instead of the older one being overwritten as though nothing had moved.
The
-j2overlap hazard described further down that comment is unchanged:halving each peak halves the sums too, but only in the position that is not the
default.
MIGRATION.mdgets one correction in the same commit: it said "365 of 365relevant
impl/cudaupload sites use the synchronouscopyToDevice", a figureinherited from #94. On current
mainit is 363 of 391 —distance_ {euclidean,l1,mesh}.hnow usecopyToDeviceAsync. The point stands for almostevery op; the number was stale.
The benchmark that would actually settle the axis is still #144.
Generated by Claude Code