Skip to content

Commit

Permalink
fixed contact destruction perf
Browse files Browse the repository at this point in the history
  • Loading branch information
@erincatto
erincatto committed Aug 4, 2023
1 parent d398526 commit 0466073
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 46 deletions.
14 changes: 5 additions & 9 deletions src/body.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,16 +161,12 @@ void b2World_DestroyBody(b2BodyId bodyId)
}

// Remove from awake contact array
// TODO_ERIN perf problem?
int32_t awakeContactCount = b2Array(world->awakeContactArray).count;
for (int32_t i = 0; i < awakeContactCount; ++i)
int32_t awakeIndex = world->contactAwakeIndexArray[contactIndex];
if (awakeIndex != B2_NULL_INDEX)
{
B2_ASSERT(i != 0);
if (world->awakeContactArray[i] == contactIndex)
{
b2Array_RemoveSwap(world->awakeContactArray, i);
break;
}
B2_ASSERT(0 <= awakeIndex && awakeIndex < b2Array(world->awakeContactArray).count);
world->awakeContactArray[awakeIndex] = B2_NULL_INDEX;
world->contactAwakeIndexArray[contactIndex] = B2_NULL_INDEX;
}

// Remove pair from set
Expand Down
51 changes: 30 additions & 21 deletions src/contact.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

// Contacts and determinism
// A deterministic simulation requires contacts to exist in the same order in b2Island no matter the thread count.
// The order must reproduce from run to run. This is necessary because Gauss-Seidel is order dependent.
// The order must reproduce from run to run. This is necessary because the Gauss-Seidel constraint solver is order dependent.
//
// Creation:
// - Contacts are created using results from b2UpdateBroadPhasePairs
Expand All @@ -35,7 +35,8 @@
// - Awake contacts are solved in parallel and they generate contact state changes.
// - These state changes may link islands together using union find.
// - The state changes are ordered using a bit array that encompasses all contacts
// - As long as contacts are created in deterministic order, island link order is deterministic
// - As long as contacts are created in deterministic order, island link order is deterministic.
// - This keeps the order of contacts in islands deterministic

// Friction mixing law. The idea is to allow either fixture to drive the friction to zero.
// For example, anything slides on ice.
Expand Down Expand Up @@ -132,6 +133,8 @@ void b2CreateContact(b2World* world, b2Shape* shapeA, b2Shape* shapeB)
b2Contact* contact = (b2Contact*)b2AllocObject(&world->contactPool);
world->contacts = (b2Contact*)world->contactPool.memory;

int32_t contactIndex = contact->object.index;

contact->flags = b2_contactEnabledFlag;

if (shapeA->isSensor || shapeB->isSensor)
Expand Down Expand Up @@ -159,7 +162,7 @@ void b2CreateContact(b2World* world, b2Shape* shapeA, b2Shape* shapeB)
contact->edges[0].prevKey = B2_NULL_INDEX;
contact->edges[0].nextKey = bodyA->contactList;

int32_t keyA = (contact->object.index << 1) | 0;
int32_t keyA = (contactIndex << 1) | 0;
if (bodyA->contactList != B2_NULL_INDEX)
{
b2Contact* contactA = world->contacts + (bodyA->contactList >> 1);
Expand All @@ -176,7 +179,7 @@ void b2CreateContact(b2World* world, b2Shape* shapeA, b2Shape* shapeB)
contact->edges[1].prevKey = B2_NULL_INDEX;
contact->edges[1].nextKey = bodyB->contactList;

int32_t keyB = (contact->object.index << 1) | 1;
int32_t keyB = (contactIndex << 1) | 1;
if (bodyB->contactList != B2_NULL_INDEX)
{
b2Contact* contactB = world->contacts + (bodyB->contactList >> 1);
Expand All @@ -187,18 +190,28 @@ void b2CreateContact(b2World* world, b2Shape* shapeA, b2Shape* shapeB)
bodyB->contactCount += 1;
}

if (b2IsBodyAwake(world, bodyA) || b2IsBodyAwake(world, bodyB))
// A contact should only be created from an awake body
B2_ASSERT(b2IsBodyAwake(world, bodyA) || b2IsBodyAwake(world, bodyB));

int32_t awakeIndex = b2Array(world->awakeContactArray).count;
b2Array_Push(world->awakeContactArray, contactIndex);

if (contactIndex == b2Array(world->contactAwakeIndexArray).count)
{
b2Array_Push(world->contactAwakeIndexArray, awakeIndex);
}
else
{
// A contact does not need to be in an island to be awake.
b2Array_Push(world->awakeContactArray, contact->object.index);
B2_ASSERT(contactIndex < b2Array(world->contactAwakeIndexArray).count);
world->contactAwakeIndexArray[contactIndex] = awakeIndex;
}

// Add to pair set for fast lookup
uint64_t pairKey = B2_SHAPE_PAIR_KEY(contact->shapeIndexA, contact->shapeIndexB);
b2AddKey(&world->broadPhase.pairSet, pairKey);
}

void b2DestroyContact(b2World* world, b2Contact* contact, bool removeAwake)
void b2DestroyContact(b2World* world, b2Contact* contact)
{
// Remove pair from set
uint64_t pairKey = B2_SHAPE_PAIR_KEY(contact->shapeIndexA, contact->shapeIndexB);
Expand Down Expand Up @@ -253,7 +266,9 @@ void b2DestroyContact(b2World* world, b2Contact* contact, bool removeAwake)
nextEdge->prevKey = edgeB->prevKey;
}

int32_t edgeKeyB = (contact->object.index << 1) | 1;
int32_t contactIndex = contact->object.index;

int32_t edgeKeyB = (contactIndex << 1) | 1;
if (bodyB->contactList == edgeKeyB)
{
bodyB->contactList = edgeB->nextKey;
Expand All @@ -267,19 +282,13 @@ void b2DestroyContact(b2World* world, b2Contact* contact, bool removeAwake)
}

// Remove from awake contact array
if (removeAwake)
b2Array_Check(world->contactAwakeIndexArray, contactIndex);
int32_t awakeIndex = world->contactAwakeIndexArray[contactIndex];
if (awakeIndex != B2_NULL_INDEX)
{
// TODO_ERIN add awake index back to b2Contact to speed this up
int32_t contactIndex = contact->object.index;
int32_t awakeContactCount = b2Array(world->awakeContactArray).count;
for (int32_t i = 0; i < awakeContactCount; ++i)
{
if (world->awakeContactArray[i] == contactIndex)
{
b2Array_RemoveSwap(world->awakeContactArray, i);
break;
}
}
B2_ASSERT(0 <= awakeIndex && awakeIndex < b2Array(world->awakeContactArray).count);
world->awakeContactArray[awakeIndex] = B2_NULL_INDEX;
world->contactAwakeIndexArray[contactIndex] = B2_NULL_INDEX;
}

b2FreeObject(&world->contactPool, &contact->object);
Expand Down
5 changes: 4 additions & 1 deletion src/contact.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ typedef struct b2Contact

uint32_t flags;

// This is too hot and has been moved to a separate array
//int32_t awakeIndex;

b2ContactEdge edges[2];

int32_t shapeIndexA;
Expand All @@ -87,7 +90,7 @@ typedef struct b2Contact
void b2InitializeContactRegisters(void);

void b2CreateContact(b2World* world, b2Shape* shapeA, b2Shape* shapeB);
void b2DestroyContact(b2World* world, b2Contact* contact, bool removeAwake);
void b2DestroyContact(b2World* world, b2Contact* contact);

bool b2ShouldShapesCollide(b2Filter filterA, b2Filter filterB);

Expand Down
2 changes: 1 addition & 1 deletion src/joint.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ static void b2DestroyContactsBetweenBodies(b2World* world, b2Body* bodyA, b2Body
if (contact->edges[otherEdgeIndex].bodyIndex == otherBodyIndex)
{
// Careful, this removes the contact from the current doubly linked list
b2DestroyContact(world, contact, true);
b2DestroyContact(world, contact);
}
}
}
Expand Down
43 changes: 29 additions & 14 deletions src/world.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ b2WorldId b2CreateWorld(const b2WorldDef* def)
world->splitIslandArray = b2CreateArray(sizeof(int32_t), B2_MAX(def->bodyCapacity, 1));

world->awakeContactArray = b2CreateArray(sizeof(int32_t), B2_MAX(def->contactCapacity, 1));
world->contactAwakeIndexArray = b2CreateArray(sizeof(int32_t), world->contactPool.capacity);

world->splitIslandIndex = B2_NULL_INDEX;
world->stepId = 0;
Expand Down Expand Up @@ -179,6 +180,7 @@ void b2DestroyWorld(b2WorldId id)
b2DestroyArray(world->awakeContactArray, sizeof(int32_t));

b2DestroyArray(world->awakeIslandArray, sizeof(int32_t));
b2DestroyArray(world->contactAwakeIndexArray, sizeof(int32_t));
b2DestroyArray(world->splitIslandArray, sizeof(int32_t));

b2Island* islands = world->islands;
Expand Down Expand Up @@ -219,6 +221,7 @@ static void b2CollideTask(int32_t startIndex, int32_t endIndex, uint32_t threadI
b2Contact* contacts = world->contacts;
int32_t awakeCount = b2Array(world->awakeContactArray).count;
int32_t* awakeContactArray = world->awakeContactArray;
int32_t* contactAwakeIndexArray = world->contactAwakeIndexArray;

B2_MAYBE_UNUSED(awakeCount);
B2_ASSERT(startIndex < endIndex);
Expand All @@ -227,16 +230,21 @@ static void b2CollideTask(int32_t startIndex, int32_t endIndex, uint32_t threadI
for (int32_t awakeIndex = startIndex; awakeIndex < endIndex; ++awakeIndex)
{
int32_t contactIndex = awakeContactArray[awakeIndex];
if (contactIndex == B2_NULL_INDEX)
{
// Contact was destroyed
continue;
}

B2_ASSERT(0 <= contactIndex && contactIndex < world->contactPool.capacity);
b2Contact* contact = contacts + contactIndex;

// B2_ASSERT(contact->awakeIndex == awakeIndex);
B2_ASSERT(contactAwakeIndexArray[contactIndex] == awakeIndex);
B2_ASSERT(contact->object.index == contactIndex && contact->object.index == contact->object.next);

// Reset contact awake index. Contacts must be added to the awake contact array
// each time step in the island solver.
// contact->awakeIndex = B2_NULL_INDEX;
contactAwakeIndexArray[contactIndex] = B2_NULL_INDEX;

b2Shape* shapeA = shapes + contact->shapeIndexA;
b2Shape* shapeB = shapes + contact->shapeIndexB;
Expand Down Expand Up @@ -291,22 +299,13 @@ static void b2UpdateTreesTask(int32_t startIndex, int32_t endIndex, uint32_t thr
b2TracyCZoneEnd(tree_task);
}

int32_t b2_awakeContactCount;

static void b2Collide(b2World* world)
{
B2_ASSERT(world->workerCount > 0);

int32_t awakeContactCount = b2Array(world->awakeContactArray).count;
b2_awakeContactCount = awakeContactCount;

if (awakeContactCount == 0)
{
return;
}

b2TracyCZoneNC(collide, "Collide", b2_colorDarkOrchid, true);

// Rebuild the collision tree for dynamic and kinematic bodies to keep their query performance good.
if (b2_parallel)
{
world->userTreeTask = world->enqueueTaskFcn(&b2UpdateTreesTask, 1, 1, world, world->userTaskContext);
Expand All @@ -317,6 +316,13 @@ static void b2Collide(b2World* world)
world->userTreeTask = NULL;
}

int32_t awakeContactCount = b2Array(world->awakeContactArray).count;

if (awakeContactCount == 0)
{
return;
}

for (uint32_t i = 0; i < world->workerCount; ++i)
{
b2SetBitCountAndClear(&world->taskContextArray[i].contactStateBitSet, awakeContactCount);
Expand Down Expand Up @@ -358,12 +364,14 @@ static void b2Collide(b2World* world)
B2_ASSERT(awakeIndex < (uint32_t)awakeContactCount);

int32_t contactIndex = world->awakeContactArray[awakeIndex];
B2_ASSERT(contactIndex != B2_NULL_INDEX);

b2Contact* contact = world->contacts + contactIndex;

if (contact->flags & b2_contactDisjoint)
{
// Bounding boxes no longer overlap
b2DestroyContact(world, contact, false);
b2DestroyContact(world, contact);
}
else if (contact->flags & b2_contactStartedTouching)
{
Expand Down Expand Up @@ -751,6 +759,8 @@ static void b2Solve(b2World* world, b2StepContext* context)

b2Array_Clear(world->awakeContactArray);

int32_t* contactAwakeIndexArray = world->contactAwakeIndexArray;

// Iterate the bit set
// The order of the awake contact array doesn't matter, but I don't want duplicates. It is possible
// that body A or body B or both bodies wake the contact.
Expand All @@ -765,7 +775,12 @@ static void b2Solve(b2World* world, b2StepContext* context)
uint32_t ctz = b2CTZ(word);
uint32_t contactIndex = 64 * k + ctz;

// TODO_ERIN consider adding the awake index to the contact to speed up deletes
B2_ASSERT(contactAwakeIndexArray[contactIndex] == B2_NULL_INDEX);

// This cache miss is brutal but is necessary to make contact destruction reasonably quick.
contactAwakeIndexArray[contactIndex] = b2Array(world->awakeContactArray).count;

// This is fast
b2Array_Push(world->awakeContactArray, contactIndex);

// Clear the smallest set bit
Expand Down
3 changes: 3 additions & 0 deletions src/world.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ typedef struct b2World
// but a bit set is used to prevent duplicates
int32_t* awakeContactArray;

// Hot data split from b2Contact
int32_t* contactAwakeIndexArray;

// This transient array holds islands created from splitting a larger island.
int32_t* splitIslandArray;

Expand Down

0 comments on commit 0466073

Please sign in to comment.