Skip to content

Collision detection performance: flat pair cache, no worker spin, wider threading - #7703

Draft
The-E wants to merge 4 commits into
scp-fs2open:masterfrom
The-E:experimental/collision-improvements
Draft

Collision detection performance: flat pair cache, no worker spin, wider threading#7703
The-E wants to merge 4 commits into
scp-fs2open:masterfrom
The-E:experimental/collision-improvements

Conversation

@The-E

@The-E The-E commented Aug 11, 2026

Copy link
Copy Markdown
Member

Summary

This branch makes collision detection faster. It does three things:

  1. It replaces the collision pair cache with a flat hash table.
  2. It removes a busy-wait that kept all collision worker threads at 100% CPU.
  3. It gives multithreaded support to debris, asteroid, and weapon-to-weapon collisions.

It also adds a benchmark mode. The benchmark mode makes the results repeatable.

Measurements

The test build is RelWithDebInfo. The test mission is bp2-massivebattle from Blue Planet
Complete 3.3.3. Each run measures 600 frames after a warmup of 120 frames.

Configuration Collision phase
Before, single thread 5.28 ms/frame
Before, -threads 8 4.95 ms/frame
After, -threads 8 3.41 ms/frame

The 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.

Configuration Collision phase
Single thread 1.32 ms/frame
-threads 8 0.91 ms/frame

The changes

1. Flat collision pair cache

Collision_cached_pairs was an SCP_unordered_map. That container puts each entry in its own
heap 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_pair is now 12
bytes instead of 40 bytes, because the key already contains both object numbers.

Note one behavior change. SCP_unordered_map keeps node addresses stable across insertions. An
open-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 again
after the call.

2. Worker threads no longer spin

spin_up_mp_collision() ran before the broadphase. However, only the last sweep pass sends work
to 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 condition
variable. 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 _check function and a main-thread _process function:

  • debris to weapon
  • debris to ship
  • asteroid to weapon
  • asteroid to ship
  • weapon to weapon

Two problems were found and corrected first:

  • debris_check_collision() wrote wp->collisionInfo = new mc_info directly. The function now
    has an optional out_mc parameter. 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 a
    worker 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 for
each 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 the
game.

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 LoggingEnabled is false in
NDEBUG builds.

The counters are in code/object/collideprofile.{h,cpp}. They use thread-local storage and no
atomics. The tracing system cannot do this work, because tracing::complete::start() increments
a current_id variable that is not atomic.

6. Correction to collide_asteroid_prop()

collide_asteroid_prop() called prop_check_collision(prop_objp, prop_objp, ...). It sent the
prop 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() uses other_obj only in two tests for
OBJ_SHIP. A prop and an asteroid both fail these tests, and the collision geometry comes from
prop_hit_info->heavy and prop_hit_info->light, which the caller sets correctly. Therefore
the collision result was correct. The code was still incorrect, and it could fail after a future
change to those tests.

Verification

  • All 228 unit tests pass.
  • A Debug build runs bp2-massivebattle with -threads 8 and gives no assertion failures. This
    run sent 27,817 pairs for each frame to the worker threads.
  • A Debug build runs Solaris mission 2 with -threads 8 and gives no assertion failures. This
    run 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

  1. The prop collision types still use one thread. They are rare.
  2. The event counters are off by default. COLLISION_PROFILING in collideprofile.h is 0.
    Set it to 1 to get the counts. The counts cost approximately 5% of the collision time. The
    per-frame phase timers and the -collision_bench option do not need the counters.

Known limitation

pair gen/cull is now approximately 2.3 ms of the 3.41 ms collision phase. This work is on the
main thread. obj_collide_pair() is called approximately 59,000 times for each frame with pairs
that fail the first tests. This is the next area to improve.

🤖 Generated with Claude Code

The-E and others added 4 commits August 9, 2026 13:08
…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 BMagnu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants