Memory Managing in Bullet & a few suggestions

User avatar
cippyboy
Posts: 36
Joined: Fri Aug 25, 2006 1:00 am
Location: Bucharest

Memory Managing in Bullet & a few suggestions

Post by cippyboy »

Well, I finally upgraded to 2.42b and I'm impressed that I didn't had to rename the btVector3 and that there's less memory leaks (pointers not deallocated) but... since I like to eliminate ALL memory leaks, I'd like to indicate some clear bugs that I found.

First, the code for memory leaking that I use under VC8 (the built-in one, it even detects leaks in constructors)

Code: Select all

//The MemoryLeak Detector
#define _CRTDBG_MAP_ALLOC
#include <stdlib.h>
#include <crtdbg.h>

void main()
{
//...
_CrtSetDbgFlag ( _CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF );
//...
}
Now, I've isolated an allocation that gives 2 memory leaks and it's

Code: Select all

Dispatcher=new	btCollisionDispatcher();
that gives

Detected memory leaks!
Dumping objects ->
{1552} normal block at 0x00876080, 4 bytes long.
Data: <| > 7C 97 DE 00
{1551} normal block at 0x00877AE8, 400 bytes long.
Data: < > CD CD CD CD CD CD CD CD CD CD CD CD CD CD CD CD
Object dump complete.


The rest happens when I add bodies in the btDynamicsWorld. With just one dynamic object and heck load of static ones it gives

Dumping objects ->
{1614} normal block at 0x0199AFF8, 47 bytes long.
Data: < $ c > F8 AF 99 01 ED ED ED ED 24 00 00 00 BF 63 90 01
{1613} normal block at 0x0199AF88, 48 bytes long.
Data: < o > B8 6F DE 00 CD CD CD CD CD CD CD CD CD CD CD CD
{1597} normal block at 0x0199AAD8, 47 bytes long.
Data: < $ L > D8 AA 99 01 ED ED ED ED 24 00 00 00 AF 4C 90 01
{1596} normal block at 0x0199AA68, 48 bytes long.
Data: < o > B8 6F DE 00 CD CD CD CD CD CD CD CD CD CD CD CD
{1552} normal block at 0x00876080, 4 bytes long.
Data: <| > 7C 97 DE 00
{1551} normal block at 0x00877AE8, 400 bytes long.
Data: < > CD CD CD CD CD CD CD CD CD CD CD CD CD CD CD CD
Object dump complete.


As you can see, it includes the leaks from above. I'm hoping that one day these things are going to be solved because if one more error allocation happens (that belongs to me) I wouldn't know if it's from Bullet or if it's because of my buggy code so I have to turn off Bullet for those occasions and by turning it off the bug may not appear again so now I'm just crossing my fingers :D

Also, a few suggestions for the decrease of allocations on the user side, why not turn this

Code: Select all

         Dispatcher=new	btCollisionDispatcher();	
	Broadphase=new btSimpleBroadphase;
	//default constraint solver
	Solver=new btSequentialImpulseConstraintSolver;
	DynamicsWorld=new btDiscreteDynamicsWorld(Dispatcher,Broadphase,Solver);

Code: Select all

into this
	#define SIMPLE_DISPATCHER  0x001
	#define COMPLEX_DISPATCHER 0x002

	#define SIMPLE_BROADPHASE  0x010
	#define COMPLEX_BROADPHASE 0x011

	#define SIMPLE_SOLVER	   0x100
	#define COMPLEX_SOLVER     0x101

	DynamicsWorld=new btDiscreteDynamicsWorld(SIMPLE_DISPATCHER,
		SIMPLE_BROADPHASE,SIMPLE_SOLVER);
        //then allocate them in the constructor for btDiscreteDynamicsWorld
because usually people want something simple to start out with and don't like to bother with complicate allocations or just want some physics to kick in there and don't mind about what's going on in the inside (like me :D), not forgetting the fact that I have to keep all those variables somewhere so I can deallocate them (not like in the demos I've seen).

But hey, you may probably say that allocations aren't the primary issue right now so... at least I said it :)
User avatar
Erwin Coumans
Site Admin
Posts: 4221
Joined: Sun Jun 26, 2005 6:43 pm
Location: California, USA

Post by Erwin Coumans »

Hi,

The BasicDemo shows an example how to properly delete all memory. The other demos indeed need to be cleaned up.

A helper function for creation can be added, but no in the constructor of btDiscreteDynamicsWorld. That create undesireable dependencies.

Thanks for the feedback,
Erwin
User avatar
cippyboy
Posts: 36
Joined: Fri Aug 25, 2006 1:00 am
Location: Bucharest

Post by cippyboy »

I never saw that deallocation code, ups!

//10 minutes later

Ok, so the code does exactly what I do, and now I added that collision object deletion thing... I was just deleting the RigidBody created with something like localCreateRigidBody(), removing the rigidbody or removing the collision object, isn't that the same thing ?

//10 mins later

Well, I tested with removing the collision object first, then rigid body... gives a heck load of leaks (RigidBody is derived from CollisionObject I think?); the other way around just gives the initial leaks

//20 mins later

Hm... nailed some leaks coming from the convex meshes, apparently I was forgetting about the allocation of a btTriangleIndexVertexArray that I didn't stored a pointer to delete later... sorry about that, but! I still have those 2 errors

Code: Select all

Detected memory leaks!
Dumping objects ->
{1552} normal block at 0x00876990, 4 bytes long.
 Data: <    > 9C A4 DE 00 
{1551} normal block at 0x00877A48, 400 bytes long.
 Data: <                > 03 00 00 00 CD CD CD CD CD CD CD CD CD CD CD CD 
Object dump complete.
coming from straight this line

Code: Select all

Dispatcher=new	btCollisionDispatcher();
And I ran the basic demo with the leak detector on, oh man, I couldn't even count the leaks :D
User avatar
Erwin Coumans
Site Admin
Posts: 4221
Joined: Sun Jun 26, 2005 6:43 pm
Location: California, USA

Post by Erwin Coumans »

As a rule, just delete what you created with 'new', in the reverse order. Check out the BasicDemo how to do this, but enable 'CHECK_MEMORY_LEAKS'.
cippyboy wrote:

Code: Select all

Dispatcher=new	btCollisionDispatcher();
And I ran the basic demo with the leak detector on, oh man, I couldn't even count the leaks :D
You need to delete the dispatcher. Also, make sure to define

Code: Select all

#define CHECK_MEMORY_LEAKS 1
at the top of the BasicDemo: Glut programs don't exit properly, so memory leak tools won't work.

Also, one memory leak I've found in BasicDemo is lack of deletion of motionState (apart from some glut related leaks).
You can delete the motionState for each rigidbody with the following snippet:

Code: Select all

void	BasicDemo::exitPhysics()
{


	//cleanup in the reverse order of creation/initialization

	//remove the rigidbodies from the dynamics world and delete them
	int i;
	for (i=m_dynamicsWorld->getNumCollisionObjects()-1; i>=0 ;i--)
	{
		btRigidBody* obj = (btRigidBody*)m_dynamicsWorld->getCollisionObjectArray()[i];
		btMotionState* myMotionState = obj->getMotionState();
		m_dynamicsWorld->removeCollisionObject( obj );
		delete obj;
		delete myMotionState;
	}

....

Hope this helps,
Erwin
User avatar
cippyboy
Posts: 36
Joined: Fri Aug 25, 2006 1:00 am
Location: Bucharest

Post by cippyboy »

I'm not trying to be an ass but I was already doing that :D

instead of that huge loop in which you wore removing+deleting I was having the remove+delete operation inside each object(for which I store body,motion state shape and vertex array information) and I have these codes for deallocation

Code: Select all

//a template for deleting, if there is something to delete
template <class T>
bool SafeDelete2(T **p)
{
	if (*p)
	{
		delete *p;
		*p=NULL;
		return true;
	}
	else 
		return false;
}
//for each object
{
DynamicsWorld->removeRigidBody(body);	
//
SafeDelete2(&body);
SafeDelete2(&MotionState);
SafeDelete2(&Shape);
//only for polygonal meshes	
SafeDelete2(&Array);
}


//and the for the whole system
SafeDelete2(&DynamicsWorld);
SafeDelete2(&Solver);
SafeDelete2(&Broadphase);
SafeDelete2(&Dispatcher);
Is there anything wrong with it ?
Anyway, I'll look on different types of Dispatcher/Solver/Broadphase/world allocations, maybe It's something wrong with that.

//10 mins later
FOUND IT !
I found the problem, I was using this:

Code: Select all

Dispatcher=new	btCollisionDispatcher();
it works but has 2 allocation errors;
if using this

Code: Select all

Dispatcher=new	btCollisionDispatcher(true);
there's no allocation error but my ball just passes through everything(because I don't add collision functions, unlike the basic demo).
Maybe it's something with those collision functions ? Or I do something wrong ?
User avatar
Erwin Coumans
Site Admin
Posts: 4221
Joined: Sun Jun 26, 2005 6:43 pm
Location: California, USA

Post by Erwin Coumans »

cippyboy wrote:I found the problem, I was using this:

Code: Select all

Dispatcher=new	btCollisionDispatcher();
it works but has 2 allocation errors;
Which 2 allocation errors exactly?

The collision functions are deleted in the destructor ~btCollisionDispatcher.

Erwin
User avatar
cippyboy
Posts: 36
Joined: Fri Aug 25, 2006 1:00 am
Location: Bucharest

Post by cippyboy »

It's something like this

Code: Select all

Detected memory leaks!
Dumping objects ->
{826} normal block at 0x0196A680, 4 bytes long.
 Data: <    > 84 97 DE 00 
{825} normal block at 0x00877120, 400 bytes long.
 Data: <                > 01 00 00 00 CD CD CD CD CD CD CD CD CD CD CD CD 
Object dump complete.
but what is exactly 400 bytes and gets allocated on every physics world I have ? From the looks of it, there's a variable on 4 bytes with the value 1 and the rest is just plain uninitialised data. Looking at what I posted above, that value is sometimes 3 and sometimes uninitialised.