Improve CUDA module performance and CUDA 12/13 build compatibility#15
Merged
Conversation
- knn: remove per-thread in-kernel cudaMalloc/cudaFree, add k-th-best guard, incremental shell counting, CSR sentinel (OOB read fix), Build() re-entry leak fix (kNN query: 121ms -> 56ms, bit-identical) - image: persistent grow-only workspaces for BoxFilterCuda and the free-function normals path (3.8x / 1.8x), create texture object once in NormalComputerCuda::Init (was leaking one per call), zero-init d_normals/d_points (uninitialized border pixels leaked garbage into outputs and TSDF fusion) - mesh/voxel: move per-call thrust temporaries and d_changed into RemoveSmallConnectedComponentsBuf, enable label-propagation early exit by default, drop redundant mid-pipeline device syncs (ExtractMesh 2x, ComputeVertexNormals 1.7x) - CMake: make CUDA architecture defaults toolkit-aware (CUDA 13 dropped < sm_75; add sm_100/120 on 12.8+) and fix the arch guard that never fired after enable_language, leaving builds at sm_52 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The comments in image.cu, mesh.cu and voxel.h were double-mangled (Shift-JIS bytes corrupted by an earlier encoding round-trip) and unreadable in any encoding. Rewrite them in plain-ASCII English based on the surrounding code. No code changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Unlike image.cu/mesh.cu, the Japanese comments in voxel.cu were intact (valid UTF-8), so they are translated faithfully. Comment-only change; code lines are byte-identical apart from dropping the leading BOM. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
CUDA module performance refactor (stages 1-3) + CUDA 12/13 build compatibility
This PR is the first three stages of an incremental performance refactor of
src/cuda/, based on a static audit of the module. Every stage was verified on real hardware for both result consistency (bit-identical where the pipeline is deterministic, statistical equivalence where it is not) and measured speedup.Measured results
Environment: RTX PRO 6000 Blackwell (sm_120), CUDA 13.2, VS2026, Release build.
knngrid.plybit-identicalBoxFilterCuda(ex06, per call, k=51)ComputeNormalsCudaraw overload (ex06, per call)ExtractMesh(ex21)ComputeVertexNormals(ex21)RemoveSmallConnectedComponents(ex21, cold call)* ex21 calls this once, so the measurement includes first-call workspace allocation. The per-call thrust temporaries are now persistent, so warm calls no longer allocate at all — that benefit is structural and not visible in this one-shot benchmark.
Changes
Stage 1 —
knn.cu/knn.cccudaMalloc/cudaFree(the single worst issue in the module: device-heap allocation serializes all query threads and can silently exhaust the default 8 MB heap). Each thread now uses its slice of the output buffers as the working sorted list.Stage 2 —
image.cuBoxFilterCudaand the free-function normals path rebuilt their entire GPU pipeline every call (cudaMalloc3DArray+ texture object +cudaMalloc+ full teardown). Both now use grow-only persistent workspaces; redundant mid-pipelinecudaDeviceSynchronizecalls removed.NormalComputerCudaleaked one texture object perComputeNormalscall; the texture is now created once inInit(the depth array it views is persistent).Initalso releases previous resources on re-init.Stage 3 —
mesh.cu/mesh.cuh/voxel.cuRemoveSmallConnectedComponentsallocated 7 thrust device vectors plusd_changedon every call despite receiving a reuse buffer; all scratch now lives inRemoveSmallConnectedComponentsBuf.d_changedevery 8 iterations). Results are unchanged: once the propagation reaches its fixed point, remaining iterations were no-ops.ComputeVertexNormals(default-stream ordering already guarantees correctness).Build compatibility (VS2022/VS2026 x CUDA 12/13)
if(NOT DEFINED CMAKE_CUDA_ARCHITECTURES)guard sat afterenable_language(CUDA), where CMake has always already cached a detected default — so the project's multi-arch list was dead code and builds actually targeted only the compiler-default arch (e.g. sm_52). User intent is now captured beforeenable_language.75;80;86;87;89;90;100;120(CUDA 13 removed < sm_75), CUDA 12.8+ ->50...120(adds Blackwell), older 12.x ->50...90.-allow-unsupported-compileris added automatically for CUDA 12.x + MSVC >= VS2026 (non-VS-generator builds). Note: VS2026 + CUDA 12.x via the VS generator is not possible regardless (NVIDIA ships no VS2026 MSBuild integration in 12.x).knngrid.ply.Bug fixes (results legitimately change)
d_normals/d_points: the normals kernels return early for border pixels without writing them, socudaMallocgarbage leaked into the outputs. Worse, in the fusion pipeline the garbage border points passed the invalid-point test (pt == (0,0,0)skip) inFuseOrganizedPointCloudMultiKernelNaiveand contaminated the TSDF. Outputs are now zero-initialized per call; ex06's CUDA outputs became run-to-run deterministic, and ex21's ~1% mesh delta versus the old baseline is the corrected behavior.offsets[voxel + 1], but the host allocated exactlyvoxel_numentries. The offsets array now carries a sentinel (voxel_num + 1entries).KNnGridCuda::Build()leaked device buffers on re-invocation; previous buffers are now released.Verification methodology
knngrid.ply, ex06 CUDA images after fix 1) were compared by hash against pre-refactor baselines.atomicAddordering; confirmed pre-existing via a stash A/B test, 4 runs each producing distinct hashes with vertex counts varying ~±1%). It was therefore verified statistically: post-refactor vertex counts (16.6k-16.9k raw, ~15.1k cleaned) sit in the same band as pre-refactor runs (baseline cleaned: 15.2k).Notes
src/cuda/image.cuwas converted from Shift-JIS to UTF-8 in passing (its encoding broke text-level tooling). Comments that were still valid Shift-JIS were preserved as proper UTF-8; comments that were already double-mangled in the repository history remain garbled (they were unrecoverable in any encoding).FinalizeLabelAndCount/ReduceFlyingNoiseOnVoxels/ label propagation, unified stream policy replacing the remaining per-kernelcudaDeviceSynchronizecalls, and an RAII device buffer type to replace ~50 rawcudaMallocsites.🤖 Generated with Claude Code