Repository navigation
Improve candidate detection by making candidates trivially copyable - #286
Merged
Merged
Conversation
The broad phase stores millions of candidates and every one carried a vptr: EdgeEdgeCandidate was 16 bytes for 8 bytes of payload, and being polymorphic it was not trivially copyable, so the memcpy fast path in merge_thread_local_vectors was dead code on every candidate path. - `StencilMixin<Derived>` (candidates/stencil_mixin.hpp) holds the stencil operations that were spread across CollisionStencil, reaching the primitives through a CRTP self-cast. It has no data members and no virtual functions, so deriving from it costs nothing in size. `StencilMath` holds the statics, compiled once rather than once per stencil. - The five candidate types derive from the mixin rather than CollisionStencil, and are now 8 bytes and trivially copyable (PlaneVertexCandidate excepted: it holds an Eigen::Hyperplane). - `StencilAdapter` and `DTypeStencilAdapter` (candidates/stencil_adapter.hpp) give a candidate the polymorphic interface back, and the collision types derive from those. The distance type that the edge-edge, edge-vertex and face-vertex collisions cache used to be a virtual their candidate's own math called, so the math now takes it as a defaulted argument and the adapter supplies it. - `Candidates::visit()` replaces `operator[]`, which can no longer hand out a CollisionStencil&; `NormalCollisions::visit()` mirrors it so `semi_implicit_stiffness` is written once for both. - Python keeps its API: each candidate binds the stencil operations through `define_stencil_methods`, and `Candidates.__getitem__` returns the concrete type by reference, now with the container's lifetime tied to it. Edge-edge detection on the CPU, median of three runs of 20 samples: | Scene | Before | After | Speedup | |-------------------|------------|-----------|---------| | Cloth-Funnel | 0.82 ms | 0.41 ms | 1.97x | | Armadillo-Rollers | 28.16 ms | 10.79 ms | 2.61x | | Rod-Twist | 11.55 ms | 5.68 ms | 2.04x | | Cloth-Ball | 29.97 ms | 13.09 ms | 2.29x | | N-Body-Simulation | 123.28 ms | 42.14 ms | 2.93x | | Puffer-Ball | 1129.95 ms | 329.71 ms | 3.43x | LBVH build, which touches no candidates, is unchanged at 0.98-1.06x. Both stages of detection gain: on Cloth-Ball the traversal goes from 8.19 to 6.35 ms because the thread-local vectors accumulate half as many bytes, and the merge from 17.25 to 6.37 ms because it is now a memcpy. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Each thread-local buffer lands in its own slice of the output, so once a prefix sum over their sizes gives the offsets the copies are independent. The trivially-copyable path now issues one memcpy per buffer through a tbb::parallel_for instead of walking them in order. Below 1 MiB of payload the copies are short enough that the parallel_for and the two allocations its bookkeeping needs cost more than they save, so that case keeps the in-order loop. Without the size check the Two-Cubes scene, whose whole detection is 0.06 ms, regressed 2x. Cloth-Ball, mean of 10 calls, profiling build: | Stage | Before | After | |----------|--------|-------| | traverse | 6.35 | 6.15 | | merge | 6.37 | 4.90 | | total | 12.73 | 11.06 | End-to-end edge-edge detection is 1.07x (Puffer-Ball) to 1.17x (Armadillo-Rollers) faster, and 1.93x to 3.66x against the polymorphic candidates this series started from. The merge gains less than the copies alone would suggest because out.resize() value-initializes the whole buffer before the memcpys overwrite it, and that pass is still serial. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The merge sizes the output with resize() and then memcpys every element into it, so the value-initialization resize() performs first is a wasted pass: zeroing Cloth-Ball's 5.2M edge-edge candidates costs 3.3 ms of a 3.8 ms merge. DefaultInitAllocator already exists for this, and LBVH::Nodes and AABBs already use it. `CandidateVector<T>` (candidates/candidate_vector.hpp) is a std::vector with that allocator, and the broad phases now produce and consume it. merge_thread_local_vectors is templated on both allocators, so the thread-local buffers and the output no longer have to agree, and the steal in the non-trivial path is skipped when they differ. Note that clear() was never the cost: the candidates became trivially destructible when they lost their vptr, so clearing is already O(1). It is the resize that follows which was paying. Edge-edge detection, median of three runs of 20 samples: | Scene | Before | After | Speedup | |-------------------|-----------|-----------|---------| | Armadillo-Rollers | 9.22 ms | 7.03 ms | 1.31x | | Rod-Twist | 4.95 ms | 3.86 ms | 1.28x | | Cloth-Ball | 11.63 ms | 9.11 ms | 1.28x | | N-Body-Simulation | 36.31 ms | 24.86 ms | 1.46x | | Puffer-Ball | 307.75 ms | 211.28 ms | 1.46x | The merge stage itself, mean of 10 calls in a profiling build: Cloth-Ball 4.90 to 2.38 ms, Puffer-Ball 154.27 to 47.37 ms. Against the polymorphic candidates this series started from, the merge is 7.2x and 14.5x faster and detection overall is 3.29x and 5.35x. BREAKING: the broad phase detect_*_candidates() methods, Candidates' members and the candidate-taking helpers now use CandidateVector<T> rather than std::vector<T>. Elements left behind by a resize that grows the vector are uninitialized, so read only what has been written. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The device wrote the candidate ids into two parallel int32 arrays, so materializing them meant staging both on the host, copying each, and then building every candidate one at a time: 12.4 ms of Cloth-Ball's 14.7 ms detection. The candidates are trivially copyable and are two index_t in a fixed order, so with 32-bit indices a device pair and a host candidate have the same layout. The device now writes LBVH::CandidatePair, one 8-byte store per candidate rather than two into separate streams, and materialize() sizes the output and copies the set straight into it. The direct copy is used only when the filter accepts all -- otherwise the pairs still have to be trimmed on the host -- and when the layouts really do match. That is checked at compile time and confirmed once at runtime against a known pair, so reordering a candidate's members falls back to building them one by one rather than silently producing garbage. GPU detect stages, Cloth-Ball, mean of 10 calls in a profiling build: | Stage | Before | After | |-----------------------|--------|-------| | traverse | 2.15 | 2.14 | | allocate_host_buffers | 3.38 | - | | download_pairs | 2.25 | - | | construct_candidates | 6.75 | - | | download_candidates | - | 5.55 | | total | 14.65 | 7.69 | Edge-edge detection, median of three runs of 20 samples: | Scene | Before | After | Speedup | |-------------------|-----------|-----------|---------| | Cloth-Funnel | 0.20 ms | 0.17 ms | 1.20x | | Armadillo-Rollers | 12.75 ms | 8.11 ms | 1.57x | | Rod-Twist | 4.20 ms | 2.03 ms | 2.07x | | Cloth-Ball | 15.22 ms | 7.69 ms | 1.98x | | N-Body-Simulation | 59.48 ms | 25.36 ms | 2.35x | | Puffer-Ball | 472.21 ms | 205.08 ms | 2.30x | BREAKING: LBVH::DeviceCandidateView is AoS. Its `a` and `b` pointers are replaced by a single `pairs` pointer to LBVH::CandidatePair. EdgeFaceCandidate and FaceFaceCandidate gain the defaulted default constructor the other candidates already have, so they can be sized without being written. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
download_candidates undersold what the stage is: with the AoS layout there is nothing left to construct, so the whole of materialization is one device-to-host copy. copy_candidates_to_host says that, and matches how the stage benchmark's figure buckets it.
MSVC can't take the address of an overload set that merges the derived class's virtuals with StencilMixin's members through using-declarations, so py::overload_cast fails with C3889. Bind the mesh overloads with lambdas instead.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #286 +/- ##
=======================================
Coverage 96.76% 96.76%
=======================================
Files 221 223 +2
Lines 22502 22538 +36
Branches 1558 1551 -7
=======================================
+ Hits 21773 21810 +37
+ Misses 729 728 -1
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:
|
8 tasks done
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.
Description
Restructures the collision candidates into plain 8-byte structs so the broad phase can move them with
memcpyinstead of element-wise copies, touchingcandidates/,collisions/,utils/merge_thread_local.hpp, and the CUDA LBVH.StencilMixin<Derived>(CRTP, no data or virtuals) holds the stencil operations; candidates derive from it instead ofCollisionStencil, dropping the vptr.StencilAdapter/DTypeStencilAdapterrestore the polymorphic interface for the collision types.CandidateVector<T>is astd::vectorwithDefaultInitAllocator, soresize()no longer zeroes a buffer that is immediately overwritten.memcpyeach (serial below 1 MiB of payload).LBVH::CandidatePair(AoS), so the host copy is a singlememcpy.⚡ Performance
CPU, each stage of the rework (
EdgeEdgeCandidateis 8 B, down from 16 B):memcpy.GPU, single device-to-host copy:
💥 Breaking changes
Candidates::operator[]is replaced byCandidates::visit()(mirrored byNormalCollisions::visit()), since a candidate can no longer be returned as aCollisionStencil&.detect_*_candidates(),Candidatesmembers, and candidate-taking helpers useCandidateVector<T>instead ofstd::vector<T>. Elements added by a growingresize()are uninitialized.LBVH::DeviceCandidateViewreplaces itsaandbpointers with a singlepairspointer toLBVH::CandidatePair.Candidates.__getitem__now returns the concrete candidate type with the container's lifetime tied to it.Type of change
How Has This Been Tested?
visit()andCandidateVector)-O3 -march=native