Collision detection performance: flat pair cache, no worker spin, wider threading - #7703
Collision detection performance: flat pair cache, no worker spin, wider threading#7703The-E wants to merge 4 commits into
Conversation
…mance Introduce a new `-collision_bench` command-line argument to benchmark collision performance, and significantly optimize collision detection with caching and multi-threading improvements. Includes profiling support and adjustments to memory and synchronization to minimize overhead.
…hread processing Reorganize collision handling logic to decouple narrowphase results, enabling safer multi-threading with deferred processing. Introduced structures for collision data storage and transformed major collision types to use unified processing workflows via `collision_result`. Includes improved performance through batching in worker threads and main-thread deferment of state mutations.
…ision Set COLLISION_PROFILING to 0. The per-pair and per-BSP-node counters add approximately 5% to the collision time. The per-frame phase timers and the -collision_bench option stay available. Set the macro to 1 to get the counts. Two parts of the benchmark report gave a false result when the counters are off. The inline narrowphase timer is part of the counter set, so the split between "inline narrowph." and "pair gen/cull" showed 0.000 ms. The report now omits these two lines and shows a message instead. Also remove the cache_lookup_ns field, because no code writes to it. Correct the arguments in collide_asteroid_prop(). The function sent the prop object two times to prop_check_collision(). The second argument must be the asteroid object. The effect was small, because prop_check_collision() uses other_obj only in two tests for OBJ_SHIP, and a prop and an asteroid both fail those tests. The code was incorrect and could fail after a future change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Introduce cached directional, length, and radius values for beam segments, updated per frame. Replace costly calculations in `beam_collide_early_out()` with these precomputed values, significantly reducing overhead. Filter invalid collision objects before sorting to eliminate unnecessary processing.
BMagnu
left a comment
There was a problem hiding this comment.
The multithreaded path now processes approximately 93% of the narrowphase pairs. Before this branch it processed 9%.
I am skeptical here, unless you purpose-engineered a mission with tons of asteroids.
My tests in Icarus and FotG had over 90% of frametime on ship-weapon collision, and the rest almost exclusively ship-ship.
With the only thing in some missions making a dent being ship-beam collisions, which is the one thing you didn't parallelize.
Maybe bp2-massivebattle is just special there?
collision_stage_flush_size = 256
That is very large, and in a lot of cases, larger than the mission itself will have.
For more costly collisions involving ships, even a dozen collisions will already benefit from going onto three cores, rather than being batched into one. So this seems very counter-intuitive unless benchmarking the heaviest missions out there.
Worker threads no longer spin
Have you separately benchmarked what this change does to frametimes?
Spin-waiting here was a conscious choice, since we're a game engine. We do not care if we block the CPU, we care about the best possible latency here (especially here in the collision code), with the least amount of scheduling interference as possible.
Benchmark mode:
While useful, especially the cache change needs a really long test to see how it scales temporally. So there needs to be a test at least as long as icarus, if not longer to see if there's degrading in a 20 minute mission. You may want to set something up along these lines and leave it running with a benchmark script for 20 minutes to observe long term performance implications.
In general, this is the type of PR that needs broader measurements (and notably also some that profile total frametime, just in case your fine-grain benchmarks missed something, or if the changes had secondary effects not profiled here).
Specifically, I'd like to see comparisons in the style I did for #7529 between new 1-thread and old 1-thread, new 8-thread and old 8-thread, and new 1-thread and new 8-thread, on both a heavy duty mission such as massivebattle, and a more nuanced, common mission like icarus. I can give you the benchmark script if you want.
Summary
This branch makes collision detection faster. It does three things:
It also adds a benchmark mode. The benchmark mode makes the results repeatable.
Measurements
The test build is RelWithDebInfo. The test mission is
bp2-massivebattlefrom Blue PlanetComplete 3.3.3. Each run measures 600 frames after a warmup of 120 frames.
-threads 8-threads 8The multithreaded path now processes approximately 93% of the narrowphase pairs. Before this
branch it processed 9%. Only beam collisions still run on the main thread.
A second mission shows the effect on asteroid fields. The mission is Solaris mission 2. Its
asteroid field has a density of 2000.
-threads 8The changes
1. Flat collision pair cache
Collision_cached_pairswas anSCP_unordered_map. That container puts each entry in its ownheap node. Therefore each lookup caused two cache misses. A heavy mission keeps approximately
250,000 pairs. The lookup alone used approximately 44% of the collision time.
The new container is an open-addressed table with linear probing.
collider_pairis now 12bytes instead of 40 bytes, because the key already contains both object numbers.
Note one behavior change.
SCP_unordered_mapkeeps node addresses stable across insertions. Anopen-addressed table does not. Two call sites held a pointer to an entry across a call that runs
game logic. That logic can create an object, and object creation can call
collide_remove_weapons(), which compacts the table. Both call sites now get the entry againafter the call.
2. Worker threads no longer spin
spin_up_mp_collision()ran before the broadphase. However, only the last sweep pass sends workto the workers. The workers had no way to exit early, and they had no wait. Therefore every
worker spun for the full broadphase.
The measurement showed 20,235,909 idle spin iterations for each frame. Seven cores ran at
100% and did no work.
The fix moves the spin-up to a point immediately before the last sweep pass. The worker idle
path now waits on a condition variable. The count is now approximately 150 spins for each frame.
The main-thread drain loop uses
std::this_thread::yield(). It does not use a conditionvariable. A timed wait of 50 us made the drain 2.3 ms slower for each frame, because the loop
runs only about 30 times for each frame.
3. More collision types on worker threads
These types now have a pure
_checkfunction and a main-thread_processfunction:Two problems were found and corrected first:
debris_check_collision()wrotewp->collisionInfo = new mc_infodirectly. The function nowhas an optional
out_mcparameter. The caller does the allocation on the main thread.post_process_threaded_collisions()applied results while other workers still ran checks.Therefore
calculate_ship_ship_collision_physics()changed the velocity of a ship while aworker read the same ship. The results are now buffered and applied after spin-down.
4. Batched work handover
The first version of change 3 gave no speed increase. The measurement showed why.
queue_mp_collision()locked a worker queue for each pair. That cost approximately 0.83 us foreach pair. This cost was approximately equal to the work that moved to the worker.
The main thread now stages the pairs and sends them in blocks of 256. This decreased the number
of lock operations by a factor of approximately 30.
5. Benchmark mode
The new option is
-collision_bench N. It measures N frames, writes a report, and stops thegame.
The report shows the time for each phase and the counts for each event. The counts are the more
useful signal, because the mission is not repeatable frame for frame.
The report goes to stdout. It does not go to the log, because
LoggingEnabledis false inNDEBUG builds.
The counters are in
code/object/collideprofile.{h,cpp}. They use thread-local storage and noatomics. The tracing system cannot do this work, because
tracing::complete::start()incrementsa
current_idvariable that is not atomic.6. Correction to collide_asteroid_prop()
collide_asteroid_prop()calledprop_check_collision(prop_objp, prop_objp, ...). It sent theprop object two times. The second argument must be the asteroid object. The other two callers of
this function send the other object correctly.
The effect was small.
prop_check_collision()usesother_objonly in two tests forOBJ_SHIP. A prop and an asteroid both fail these tests, and the collision geometry comes fromprop_hit_info->heavyandprop_hit_info->light, which the caller sets correctly. Thereforethe collision result was correct. The code was still incorrect, and it could fail after a future
change to those tests.
Verification
bp2-massivebattlewith-threads 8and gives no assertion failures. Thisrun sent 27,817 pairs for each frame to the worker threads.
-threads 8and gives no assertion failures. Thisrun sent 679 pairs for each frame to the worker threads.
A Debug build is necessary for this test. Assertions are active only in a Debug build.
Points for review
COLLISION_PROFILINGincollideprofile.his0.Set it to
1to get the counts. The counts cost approximately 5% of the collision time. Theper-frame phase timers and the
-collision_benchoption do not need the counters.Known limitation
pair gen/cullis now approximately 2.3 ms of the 3.41 ms collision phase. This work is on themain thread.
obj_collide_pair()is called approximately 59,000 times for each frame with pairsthat fail the first tests. This is the next area to improve.
🤖 Generated with Claude Code