Disgusting RigidShape performance in debug build (solved, I think)

EDIT: Scroll all the way down for the resolution - there's no real need to see the process unless you like watching someone go slowly insane (but there's a happy end!).

My debug build has suddenly started acting up - it's performing extremely slowly, say around 1FPS. This is absurd, since I can run the exact same scene (a terrain, some replicators and static shapes - nothing intense at all) under a release build with 80FPS - and most modern games with high settings at an acceptable FPS. I hadn't noticed any slowness for a while - I had mostly been running release builds though, but I think I would have tried out the debug exe often enough to notice a hit that bad.
Is there anything suspicious in this profiler dump?

EDIT: large unnecessary profile dump snipped

It does seem like AdvanceObjects is pretty heavy - but I can't understand why. I've added two raycasts per tick to the Player class, and I know raycasts do cause problems in debug... but two per tick? It shouldn't be enough to reduce the computer to tears.


EDIT: Oh... the RigidShapes :P. There were a few of those in the scene, actually (six, to be precise). Seems Rigid isn't profiled, and nor are the RigidShape/Vehicle classes. Hmm. So that would be what's causing the huge CPU usage in AdvanceObjects. However, why is the performance hit so huge in a debug build? Especially since all of thise RigidShapes should have been at rest - I thought that meant they didn't compute physics.


EDIT: Roll up, roll up and watch me solve all my own problems :P. I've tracked the performance down to two things: first, RigidShape updates its convex's working list every tick, which is grossly unnecessary. So I introduced a Player-like mWorkingQueryBox, and now RigidShapes only update their collision info when they've moved significantly. Performance still isn't great, and after adding profiling to RigidShape, updateCollision is showing as the prime culprit. I'll see what I can do.

EDIT: The load seems to be split fairly evenly between the calls to mConvex.findClosestState and mConvex.getCollisionInfo. However, I just realised that the whole updateCollision routine shouldn't be happening if the Rigids are at rest... which they should be. Hmm.

EDIT: Bingo!

updateForces(dt);

   // Update collision information based on our current pos.
   bool collided = false;
   if (!mRigid.atRest && !mDisableMove && !isMounted()) 
   {
      collided = updateCollision(dt);

      // Now that all the forces have been processed, lets       
      // see if we're at rest.  Basically, if the kinetic energy of
      // the shape is less than some percentage of the energy added
      // by gravity for a short period, we're considered at rest.
      // This should really be part of the rigid class...
      if (mCollisionList.count) 
      {
         F32 k = mRigid.getKineticEnergy();
         F32 G = sRigidShapeGravity * dt;
         F32 Kg = 0.5 * mRigid.mass * G * G;
         if (k < sRestTol * Kg && ++restCount > sRestCount)
            mRigid.setAtRest();

updateForces sets mRigid.atRest to false. So there's not really any point having atRest as a condition in that next statement, since atRest is always false. Therefore updateCollision is called every tick.

Is that necessary?

Disclaimer: I did modify RigidShape in my own build, but I don't recall modifying this part. I'm going to go back to stock and see if the problem's there as well.


EDIT: Yes, the code is the same in stock (minus the part about being mounted). So it seems atRest is useless in the RigidShape class, since this is the only place it's used - and never saves any trouble. This problem might extend to the Vehicle class as well.

Now, I was going to say that it's necessary to call updateCollision every tick, since that's how Rigids react to collisions on themselves. But that's not the case - moving Rigids use applyImpulse on things they collide with. So really, updateCollision shouldn't need to be called every tick.

A little refactoring is in order...

EDIT: Forgot about contacts :P. Arg. So RigidShapes now slowly, slowly sink into the terrain... and then fall to their presumed deaths. So I guess it is actually necessary to call updateCollision every tick? Just to update contacts?

EDIT: No, because when the rigid is at rest it doesn't move. I just realised that my rigids are never actually coming to rest - they just keep forcing their way down and never staying still.

This is the code as I have it:

// Update internal forces acting on the body.
   mRigid.clearForces();
   updateForces(dt);

   // Update collision information based on our current pos.
   bool collided = false;
   if (!mDisableMove && !isMounted()) 
   {
      // Now that all the forces have been processed, lets       
      // see if we're at rest.  Basically, if the kinetic energy of
      // the shape is less than some percentage of the energy added
      // by gravity for a short period, we're considered at rest.
      // This should really be part of the rigid class...
      if (mCollisionList.count) 
      {
         F32 k = mRigid.getKineticEnergy();
         F32 G = sRigidShapeGravity * dt;
         F32 Kg = 0.5 * mRigid.mass * G * G;
         if (k < sRestTol * Kg && ++restCount > sRestCount)
            mRigid.setAtRest();
      }
      else
         restCount = 0;

      if(!mRigid.atRest)
      {
         collided = updateCollision(dt);

         // Integrate forward
         mRigid.integrate(dt);
      }
   }

As far as I can tell, that's not too far removed from the regular code, except that we have the opportunity to set ourselves at rest before we update collision. In cases where we are not at rest, my code is exactly the same, in effect, as stock code. And since I've never run into a case where the rigid was at rest, my code is blameless. :D

...so why are the shapes sinking through the ground?

:P

Could be the updateWorkingCollisionSet stuff I did - gotta go back and check that.

EDIT: No. I'm actually now thinking it's to do with mCollisionList.count. Sorry about this thread, guys... it's kind of me thinking out loud in an unnecessary way :P. Anyway, in the stock code, mCollisionList.count is zeroed at the start of every updateCollision (which happens every integration times per tick - or once per updatePos, which I guess is what matters). So in the stock code, the if-conditional checked to see if we had any collisions in that tick. If we did have collisions (some of which could be contacts), we'd then see if we could set atRest. Which was never used, because it was always overwritten almost immediately.
In my code, the conditional checks to see if we had any collisions/contacts last tick - since that data is never cleared out.

But I still don't get it... the two different orders still result in the exact same method calls and so on. Hrmph.





EDIT: I think I found it! After a long and silly process, I realised one of my changes was the culprit after all. original code:

// Update collision information based on our current pos.
   bool collided = false;
   if (!mRigid.atRest && !mDisableMove) 
   {
      collided = updateCollision(dt);

      // Now that all the forces have been processed, lets       
      // see if we're at rest.  Basically, if the kinetic energy of
      // the shape is less than some percentage of the energy added
      // by gravity for a short period, we're considered at rest.
      // This should really be part of the rigid class...
      if (mCollisionList.count) 
      {
         F32 k = mRigid.getKineticEnergy();
         F32 G = sRigidShapeGravity * dt;
         F32 Kg = 0.5 * mRigid.mass * G * G;
         if (k < sRestTol * Kg && ++restCount > sRestCount)
            mRigid.setAtRest();
      }
      else
         restCount = 0;
   }

   // Integrate forward
   if (!mRigid.atRest && !mDisableMove)
      mRigid.integrate(dt);

Note how the call to integrate is placed carefully outside the first if-clause, even though the conditions are the same. It's because we can change one of the conditions inside that first clause, so we want to re-check it before integrating.

Okaayyy. So that's why my 'fixed' code executed the same way as my 'original' code - because my original code was non-stock, and broken in a way. So I've reverted to what's in stock TGE.

...but updateCollision is still being called every tick!




THE ANSWER


EDIT: Easy as pie. I just changed updateForces so that atRest isn't set to false every time :P. I'm noticing some oddness, but so far only when messing around in the editor - which is prone to making the rigid slip through the terrain. I'm going to have a go at shooting some barrels around to see if the physics is stable enough. I've certainly seen a decent performance gain - and updateCollision is now off the profiler's most-wanted list.

EDIT: seems pretty stable. No incidences of falling through the terrain, but occasionally the shape will get to a stable resting point (not sure if it's actually being set at rest, but most likely), then a moment later warp itself to an odd orientation (usually just a few degrees off, sticking into the terrain a little). I think it might just be client-server stuff though, and probably also present in stock code.