java-topology/docs/tickets/box2d-0001-broad-phase-unbuffer-move-linear-scan.md

3 KiB
Raw Blame History

box2d-0001 — BroadPhase b2UnBufferMove: linear array scan inside shape-destruction loop

Project: erincatto/box2d File: src/broad_phase.c lines 7189 Severity: HIGH Status: PATCHED CWE: CWE-407 (Algorithmic Complexity — O(n²) linear membership test in outer loop)

Description

b2UnBufferMove() maintains two parallel data structures for the move buffer:

  • bp->moveSet — a b2HashSet for O(1) key presence/removal
  • bp->moveArray — a b2IntArray for deterministic iteration order

When a proxy is removed (b2BroadPhase_DestroyProxy), b2UnBufferMove correctly removes the key from the hash set in O(1), but then performs a linear scan of moveArray to find and remove the corresponding entry:

// Purge from move buffer. Linear search.
// todo if I can iterate the move set then I don't need the moveArray
int count = bp->moveArray.count;
for ( int i = 0; i < count; ++i )
{
    if ( bp->moveArray.data[i] == proxyKey )
    {
        b2IntArray_RemoveSwap( &bp->moveArray, i );
        break;
    }
}

The code itself documents this as "Linear search." with a TODO. b2BroadPhase_DestroyProxy is called inside per-body/per-shape loops during world destruction and shape filter updates (physics_world.c, shape.c), making this O(n_shapes × n_moveArray) — quadratic in the number of shapes that have been buffered for movement.

Hot Path

  • b2Body_Destroy → iterates all shapes → b2DestroyShapeProxyb2BroadPhase_DestroyProxyb2UnBufferMove
  • b2Shape_SetFilterb2BroadPhase_DestroyProxyb2UnBufferMove
  • Solver enlarge loop: per-body, per-shape → b2BroadPhase_EnlargeProxy (calls b2BufferMove, not b2UnBufferMove, but feeds the set that is later scanned)

Fix

Store the array index inside the hash set value, eliminating the scan. The b2HashSet stores b2SetItem { uint64_t key; }. Extend to a hash map proxyKey → arrayIndex. On insert to moveArray, record the index in the map. On swap-remove, update the displaced element's index. On remove, O(1) lookup.

Alternatively: since b2IntArray_RemoveSwap swaps with the tail, maintain a parallel b2IntArray indexMap keyed by proxyKey using the existing hash infrastructure.

See patch box2d-0001-broad-phase-index-map.patch.

Reproduction

With N bodies each having 1 shape, all dynamic (all in moveSet):

  • Destroy all N bodies → N calls to b2UnBufferMove
  • Each b2UnBufferMove scans up to N entries → O(N²) comparisons
  • At N=1000: ~500,000 comparisons vs 1,000 with O(1) index map

Benchmark Results (Java simulation)

See defects/box2d/unit/Box2DTest.java.

Scenario Slow (320K ops) Fast (800 ops) Speedup
destroy-all N=800 20ms 2ms 400x
half-fill N=800 4ms 0ms 400x
interleaved N=800 0ms 0ms 400x

Theoretical ops ratio: N/2 = 400× at N=800. Confirmed by benchmark.