GetContactCount() != actual # of contacts with 2 threads

A place to discuss everything related to Newton Dynamics.

Moderators: Sascha Willems, walaber

GetContactCount() != actual # of contacts with 2 threads

Postby agi_shi » Mon Jul 28, 2008 6:25 pm

Hi :)

I just installed beta16. I love the interface changes, I think iterating is definitely nicer than needing a callback, and processing all of the contacts at once is a much better idea than processing them 1 by 1.

Anyways, I encountered something very odd when Newton is set to 2 threads (1 thread works fine). Let's start with some code:
Code: Select all
body *bodyCast(const NewtonBody *_body)
        {
            body *ret = (body*)NewtonBodyGetUserData(_body);
            assert(ret);
            return ret;
        }

void contact::reject()
        {
            if (valid)
            {
                NewtonContactJointRemoveContact(_joint, _contact);
                valid = false;
            }
        }

contact::contact(const NewtonJoint *_joint, void *_contact):
            valid(true),
            _joint(_joint),
            _contact(_contact),
            _material(NewtonContactGetMaterial(_contact)),
            faceAttrib(NewtonMaterialGetContactFaceAttribute(_material)),
            normalSpeed(NewtonMaterialGetContactNormalSpeed(_material))
        {
            NewtonMaterialGetContactForce(_material, &force.x);
            NewtonMaterialGetContactPositionAndNormal(_material, &position.x, &normal.x);
            NewtonMaterialGetContactTangentDirections(_material, &tangent0.x, &tangent1.x);
        }

void processContactCB(const NewtonJoint *_joint, float dt, int thread)
        {

            body *body0 = bodyCast(NewtonJointGetBody0(_joint));
            body *body1 = bodyCast(NewtonJointGetBody1(_joint));

            const size_t NUM_CONTACTS = NewtonContactJointGetContactCount(_joint);
            contactList contacts;

            void *_contact = NewtonContactJointGetFirstContact(_joint);
            // iterate over each contact, set its defaults, and add it
            while (_contact)
            {

                contact *c = new contact(_joint, _contact);

                // set the default material properties

                c->elasticity(std::max(body0->material.elasticity, body1->material.elasticity));
                c->softness(std::max(body0->material.softness, body1->material.softness));

                float sFric = std::min(body0->material.sFric, body1->material.sFric);
                float kFric = std::min(body0->material.kFric, body1->material.kFric);
                c->frictionCoef(sFric, kFric, 0);
                c->frictionCoef(sFric, kFric, 1);

                c->frictionState(true, 0);
                c->frictionState(true, 1);

                contacts.push_back(c);

                _contact = NewtonContactJointGetNextContact(_joint, _contact);

            }

            assert(NUM_CONTACTS == contacts.size());

            body0->processContact(*body1, contacts);
            body1->processContact(*body0, contacts);

            // delete the contacts (don't leak because of new)
            contactList::reverse_iterator i = contacts.rbegin();
            for (; i != contacts.rend(); ++i)
                delete *i;
        }

// simple test to try out contact rejection
void body::processContact(body &o, const contactList &contacts)
        {
            //contacts.front()->reject();
        }


Very simply put, if I uncomment the contact rejection in body::processContact(), the assert in processContactCB() gets triggered randomly, usually towards when most of the bodies have settled for the most part (but not always):
NUM_CONTACTS 9
contacts.size() 7
Assertion failed: NUM_CONTACTS == contacts.size()


If I remove the assert, then I get a SEGFAULT, and the debugger points to NewtonWorldForEachJointDo() :|

Now, given that this only happens when we have more than 1 thread for Newton, I'm guessing I'll need to add a NewtonWorldCriticalSectionLock() when removing a contact - however, is this really what should be done? I thought a critical section lock was only necessary when changing data that doesn't belong to the current callback (like some unrelated body, for example).
agi_shi
 
Posts: 263
Joined: Fri Aug 17, 2007 6:54 pm

Re: GetContactCount() != actual # of contacts with 2 threads

Postby agi_shi » Mon Jul 28, 2008 6:32 pm

More info!

Code: Select all
NewtonWorldCriticalSectionLock(_world);
            {
               
                body0->processContact(*body1, contacts);
                body1->processContact(*body0, contacts);
               
            }
            NewtonWorldCriticalSectionUnlock(_world);


Yup, definitely needed a critical section lock. This fixes the crash and the assert is now never triggered, even with more than 1 thread. However, I'm wondering if this will cost me a lot of performance - my portals do lots of contact rejection and body modification, so it's pretty much needed here. :|
agi_shi
 
Posts: 263
Joined: Fri Aug 17, 2007 6:54 pm

Re: GetContactCount() != actual # of contacts with 2 threads

Postby Julio Jerez » Mon Jul 28, 2008 7:35 pm

NewtonWorldCriticalSectionLock(_world);
NewtonWorldCriticalSectionUnlock(_world);
are necessary evil. You need to use then because removing contact write read and write to memory internally.
Your code is correct however it might lock the the thread for too long, if you are doing processing other than rejection contacts.
The trick is to lock/unlock the minimum possible amount of time to avid the change that more than one core try to execute the same piece of code.
For example you can rearrange your code like this

Code: Select all
int count;
NewtonContact* deadContacts[32];
count = 0;
count += body0->processContact(*body1, contacts, &deadContacts[count]);
count += body1->processContact(*body0, contacts, &deadContacts[count]);

if (count) {
    NewtonWorldCriticalSectionLock(_world);
    for (int = i = 0; i < count; i ++)
   NetwpnDeleteContact(joint,  deadContacts[i]);

    NewtonWorldCriticalSectionUnlock(_world);
}

teh funtion prosseContact just place the unwanted contct in teh array, an dat eh end you delet the iwth on lock the will last very litle, and minimize the posibility that more tna one thread execute the same code at the same time.
Julio Jerez
Moderator
Moderator
 
Posts: 12249
Joined: Sun Sep 14, 2003 2:18 pm
Location: Los Angeles

Re: GetContactCount() != actual # of contacts with 2 threads

Postby agi_shi » Mon Jul 28, 2008 9:20 pm

Aha, I understand what you're saying. It's best to defer changes to the end of the callback and do them all at once in the shortest amount of code.

Would you happen to have some kind of list of what needs locking and what does not need locking?

Which of these need to be locked before done during the contact callback:
- contact tangent/normal acceleration: I assume not, since this is part of the material in the callback
- NewtonBodyGetVelocity(), GetOmega(), GetMatrix(), GetMassMatrix(), GetCollision(): I'm doing all of these on some external bodies during the callback, but since they are all read-only (or so I presume), do I need to lock?
- NewtonBodySetVelocity(), SetOmega(), SetMatrix(), SetMassMatrix(), SetCollision(): I'm also doing all of these on some external bodies during the callback, and since they modify data, I presume I'll definitely need to lock for these

Thanks!
agi_shi
 
Posts: 263
Joined: Fri Aug 17, 2007 6:54 pm

Re: GetContactCount() != actual # of contacts with 2 threads

Postby Julio Jerez » Mon Jul 28, 2008 11:50 pm

not of the get funtion lock/unlock
in a body set function do not need lock if they are call in a force and torque callback.
if you can then from a contact or material call back they you ne to call then form a lock / unlock block.
in a joint callback or matarial callback you do not need to use lock unlock if you are modifyind the parameter of the joint.
Julio Jerez
Moderator
Moderator
 
Posts: 12249
Joined: Sun Sep 14, 2003 2:18 pm
Location: Los Angeles

Re: GetContactCount() != actual # of contacts with 2 threads

Postby Nearga » Thu Jul 31, 2008 4:36 pm

i've found this threat when i searched "NewtonWorldCriticalSectionUnlock"
so, what for this function needs and is it nessesary in

Code: Select all
void ShowContacts (const NewtonJoint* contactJoint)
{
   // the application can implement some kind of contact debug here
   if (showContacts) {
      NewtonWorld* world;

      world = NewtonBodyGetWorld(NewtonJointGetBody0(contactJoint));
      NewtonWorldCriticalSectionLock(world);
      for (void* contact = NewtonContactJointGetFirstContact (contactJoint); contact; contact = NewtonContactJointGetNextContact (contactJoint, contact)) {
         dVector point;
         dVector normal;   

         NewtonMaterial* material;

         material = NewtonContactGetMaterial (contact);
         NewtonMaterialGetContactPositionAndNormal (material, &point.m_x, &normal.m_x);

         // if we are display debug info we need to block other threads from writing the data at the same time
         DebugDrawContact (point, normal);
      }
      NewtonWorldCriticalSectionUnlock(world);
   }
}
Nearga
 
Posts: 21
Joined: Thu Jul 31, 2008 11:47 am


Return to General Discussion

Who is online

Users browsing this forum: No registered users and 13 guests