[Patch] Reduce runtime memory allocations in btDiscreteDynamicsWorld::solveConstraints()

[GoogleCodeExporter]

Hi.

I'm developing a game for mobile platforms that uses Bullet. For these  
platforms runtime memory allocations should be kept to minimum, ideally to 
zero. By runtime allocations I mean those allocs and frees that occurs in 
simulation tick without changing simulation environment, I'm not caring much 
about setup allocations. 

Though Bullet does good caching of various data in many managers in growing 
arrays there are few places where it can still allocate and free memory every 
tick. For my environment it's about 6-8 allocations per physics tick (summing 
up to about 20-28 per rendered frame). Though it's seems not very much, I 
measured that on long simulation runs in stable environment (simulation without 
moving objects) that it gives about 1-2 fps difference for my project comparing 
original code and my patch where those allocations are replaces with growing 
containers. Of course this issue can be solved on allocator side as a whole, 
affecting all allocations in Bullet (for example, by using Loki's SmallObject 
allocator) but I think in some places  it's better to change Bullet code, so 
there will be no unnecessary allocs at all. Of course it sacrifices some memory 
(but not much it seems) in favor of speed.

Here's a patch, it changed two such places, both located in void 
btDiscreteDynamicsWorld::solveConstraints(btContactSolverInfo& solverInfo). So 
this patch affects those environments where constraints are used.

1. I made temporary container for sorted constrains a member of 
btDiscreteDynamicsWorld (m_sortedConstraints), so it only grows and never 
releases memory at runtime.

2. Also I made struct InplaceSolverIslandCallback a member of 
btDiscreteDynamicsWorld, so all its three containers will be also growing and 
this object will be reused between calls. This was a bit trickier - I split it 
initialization into two steps - its constructor receives those params that are 
not changing for World (solver, stackAlloc, dispatcher) and new function called 
Setup() receives runtime information on each solveConstraints() call, replacing 
full object construction. This brought declaration of this class into header 
file, and also additional include, I'm not very happy with this, maybe you can 
improve this.
There are also additional reserve() calls in ProcessIsland().

I hope this patch will be helpful and I'm not introducing some subtle bugs. 
Please review it and tell me what are you think about it.

Original issue reported on code.google.com by Anton.Br...@gmail.com on 7 Dec 2011 at 12:37

Attachments:

• SolveConstraintsAlloc.patch

[GoogleCodeExporter]

THe patch is applied in latest trunk, see 
http://code.google.com/p/bullet/source/detail?r=2464

+ move the class in the c++ file
+ don't use new/delete but Bullet memory allocators (btAlignedAlloc/Free)

Original comment by erwin.coumans on 12 Dec 2011 at 4:48

• Changed state: Fixed

[GoogleCodeExporter]

In the btDiscreteDynamicsWorld constructor there should be:
m_solverIslandCallback = new (mem) InplaceSolverIslandCallback 
(m_constraintSolver, m_stackAlloc, dispatcher);
instead of:
m_solverIslandCallback = new (mem) InplaceSolverIslandCallback 
(constraintSolver, m_stackAlloc, dispatcher);
in the case that constraintSolver is NULL.

Also, if you call btDiscreteDynamicsWorld::setConstraintSolver, then 
InplaceSolverIslandCallback will still have the old m_solver.

Original comment by andres.traks on 13 Dec 2011 at 9:20

Attachments:

• SetConstraintSolver.patch

[GoogleCodeExporter]

Seems that it's safer to move constraint solver setting into Setup() function 
instead of constructor, so it's always set to same as in world class?

Original comment by Anton.Br...@gmail.com on 14 Dec 2011 at 1:15

[GoogleCodeExporter]

thanks for the reminder, patch is applied in latest trunk:
http://code.google.com/p/bullet/source/detail?r=2541

Original comment by erwin.coumans on 29 Jun 2012 at 10:37