Smart Pointer confusion

A place to discuss everything related to Newton Dynamics.

Moderators: Sascha Willems, walaber

Smart Pointer confusion

Postby JoeJ » Fri Mar 31, 2023 5:31 pm

Being new to smart pointers, some questions come up, e.g. here:

Code: Select all
      ~Simulaton ()
      {
         if (ipTest) // ipTest is a regular pointer to a ndModel
         {
            world.RemoveModel(ipTest);
            //delete ipTest; // crashes - why?
         }
      }


I had to remove the delete because after the update to new Newton version it crashes.
I assume RemoveModel() already deletes the model, because i have no smart pointer left pointing to it? (could not really verify by stepping in with debugger)

This means i never have to delete stuff myself. It happens automatically.
But i could set the ipTest pointer to null eventually, to make clear the object is gone.

If i wanted to remove the model just temporally, but later re-enable it, i could do so by having a smart pointer pointing to it?

And i assume this applies to all other things as well, like bodies or joints.

Did i get this correctly?

If so, what's the benefit in practice? What kind of problem does this solve?
Currently i only see increased complexity and chance to introduce bugs, but i fail to get the advantage.

I also commented out deleting bodies and joints of a model in the deconstructor:
Code: Select all
   virtual ~InvertedPendulumTest ()
   {
      //if (bodyTorso) delete bodyTorso;  // crashes - why?
      //if (bodyFoot) delete bodyFoot;
      //if (joint) delete joint;
   }

Which raises the same questions, but kind of nested:
When does Newton delete those bodies and joints automatically?
And how do i prevent it in case i want to remove just temporally?

Currently i create objects like this and do not store the smart pointers:
Code: Select all
   template <class T>
   void AddBody (ndWorld* world, T* body)
   {
      ndSharedPtr<ndBody> ptr(body);
      world->AddBody(ptr);
   }

   template <class T>
   void AddJoint (ndWorld* world, T* joint)
   {
      ndSharedPtr<ndJointBilateralConstraint> ptr(joint);
      world->AddJoint(ptr);
   }
User avatar
JoeJ
 
Posts: 1453
Joined: Tue Dec 21, 2010 6:18 pm

Re: Smart Pointer confusion

Postby JoeJ » Tue Apr 04, 2023 6:56 pm

JoeJ wrote:Which raises the same questions, but kind of nested:
When does Newton delete those bodies and joints automatically?


My related assumptions are not right.
So here is a code example that actually showed a problem:
Code: Select all
if (ipTest)
         {
            ImGui::Begin("ipTest");
            ipTest->GUI();
            if (ImGui::Button("Reset"))
            {
               world.RemoveModel(ipTest);
               //delete ipTest; // this crashes, but if commenting out, the model or at least it's bodies are not automatically deleted.
               ipTest = 0;
               
               ipTest = new InvertedPendulumTest();
               ipTest->Create(&world, ipFX);
               ndSharedPtr<ndModel> ptr(ipTest);
               world.AddModel(ptr);

               guiLog.Print("reset...\n");
            }
            ImGui::End();
         }


The ipTest is a ndModel used to test a balancing controller.
If it tips over, i click the button to delete it and create a new one, so it's upright and in initial position.
But if i do this now with the smart pointers, the old model still floats around in the scene.

Here some more member functions of the model. I had to out comment any deleting to prevent crashes. The Create() function is like said in first post, so smart pointers are just temporary and not kept.

Code: Select all
   virtual ~InvertedPendulumTest ()
   {
      //if (bodyTorso) delete bodyTorso;  // crashes - why?
      //if (bodyFoot) delete bodyFoot;
      //if (joint) delete joint;
   }

   virtual void AddToWorld (ndWorld* const world)
   {
      AddBody (world, bodyTorso);
      AddBody (world, bodyFoot);
      AddJoint (world, joint);
   }

   virtual void RemoveFromToWorld (ndWorld* const world)
   {
      world->RemoveJoint(joint);
      world->RemoveBody(bodyTorso);
      world->RemoveBody(bodyFoot);
   }
User avatar
JoeJ
 
Posts: 1453
Joined: Tue Dec 21, 2010 6:18 pm

Re: Smart Pointer confusion

Postby JoeJ » Fri Apr 07, 2023 3:41 pm

Ok, i really needed that Reset button, so skimmed some shared pointer tutorials. But all i found was Bjarne quotes, lists of benefits i do not need, but nothing to help me. Demo sandbox also never deletes stuff, so isn't helpful either.

So i came up with the following hack. It seemingly works, but can't be sure if stuff really is deleted properly.

Please help. What i've done can't be right. ; )
Questions in the comments.

Code: Select all
class InvertedPendulumTest : public ndModel
{
   ndSharedPtr<ndBody> ptr_bodyTorso = 0;
   ndSharedPtr<ndBody> ptr_bodyFoot = 0;
   ndSharedPtr<ndJointBilateralConstraint> ptr_joint = 0;

   virtual ~InvertedPendulumTest ()
   {
      // idk how to delete, so the idea is: swap ptr, so when the temp ptr goes out of scope, it will delete the objects. Seems to work, but there must be a better way?
// EDIT: No need to delete bodies in the destructor, happens automatically.
/*
      {
         ndSharedPtr<ndJointBilateralConstraint> temp;
         ptr_joint.Swap(temp);
      }
      ...
*/
   }

virtual void AddToWorld (ndWorld* const world)
   {
      world->AddBody (ptr_bodyTorso);
      world->AddBody (ptr_bodyFoot);
      world->AddJoint (ptr_joint);
   }

   virtual void RemoveFromToWorld (ndWorld* const world)
   {
      world->RemoveJoint(*ptr_joint);
      world->RemoveBody(*ptr_bodyTorso);
      world->RemoveBody(*ptr_bodyFoot);
   }

   void Create (ndWorld *world, const ndMatrix &xform)
   {
      ptr_bodyFoot = new ndBodyDynamic();
      //...
   }

}

Code: Select all
   class Simulaton
   {
      ndWorld world;
      ndSharedPtr<InvertedPendulumTest> ipTest = 0;

      Simulaton ()
      {
            ipTest = new InvertedPendulumTest();
            ipTest->Create(&world, xform);
            world.AddModel((ndSharedPtr<ndModel>&)ipTest);
      }

      ~Simulaton ()
      {
         if (*ipTest)
         {
            ipTest->RemoveFromToWorld(&world);
            world.RemoveModel(*ipTest); // this does NOT call the RemoveFromWorld() method, so i do it manually but feels fishy

            // same swapping trick to delete, probably not needed here
            ndSharedPtr<InvertedPendulumTest> temp;
            ipTest.Swap(temp);
         }
      }

      GUI ()
      {
         if (*ipTest)
         {
            if (ImGui::Button("Reset"))
            {
               ipTest->RemoveFromToWorld(&world);
               world.RemoveModel(*ipTest);

               // here i need to delete manually, so using the swapping trick
               {
                  ndSharedPtr<InvertedPendulumTest> temp;
                  ipTest.Swap(temp);
               }

               // create a new model using the same shared ptr         
               ipTest = new InvertedPendulumTest();
               ipTest->Create(&world, xform);
               world.AddModel((ndSharedPtr<ndModel>&)ipTest);
            }
         }
      }
   }


So how do i do this the intended way?

I think you want to add a demo where bodies and models are removed and deleted, but then recreated and added again.
And eventually something to illustrate the benefit, which is still a mystery to me.

I'm probably not the only smart pointer newbie :P
Last edited by JoeJ on Sat Apr 08, 2023 2:30 am, edited 1 time in total.
User avatar
JoeJ
 
Posts: 1453
Joined: Tue Dec 21, 2010 6:18 pm

Re: Smart Pointer confusion

Postby MeltingPlastic » Fri Apr 07, 2023 4:19 pm

In general the use of shared pointers is to alieviate the concern of when/where to delete the object.

When a shared pointer falls out of scope, the object is deleted if its the last shared pointer to fall out of scope that is associated with the object.

So when formong a program. You might have multiple shared pointers to an object and you dont have to worry about which class should delete it.
MeltingPlastic
 
Posts: 237
Joined: Fri Feb 07, 2014 11:30 pm

Re: Smart Pointer confusion

Postby JoeJ » Fri Apr 07, 2023 5:14 pm

Yeah, but in other words shared pointers then are a convenience option attempting to automate memory management and hiding responsibility.
I do not understand this benefit, because i never was in a situation where those things were an actual problem for me. I'm just used to this responsibility, and my problems always have way different reasons.

But i do see the costs: One more level of indirection causing pointer chasing and cache misses at high granularity, and atomics which are used no matter if there are potential write hazards from multiple threads or not.

So the cost clearly outweights the benefit, i think.

However, likely i just have to accept this. But then at least i want to do it right.
So how do i delete an object without deleting the actual shared pointer?
All my pointers are class variables, so i can't delete the pointers.

Eventually i could make the pointers themselves dynamically allocated objects, so i could delete them. But then i would need a fixed pointer variable, pointing to a shared pointer, pointing to the final object. That's silly ofc.
Likely there exists some elegant usage pattern, but it did not appear to me.
User avatar
JoeJ
 
Posts: 1453
Joined: Tue Dec 21, 2010 6:18 pm

Re: Smart Pointer confusion

Postby Dave Gravel » Fri Apr 07, 2023 7:59 pm

Hi JoeJ,
Here, I am using the multibody vehicle model and with my reset scene, it works well. I did a quick test with a custom model and it seems to work well with my reset scene too.
The object that I created inside the class is automatically deleted when I reset the scene and remove the model.

Code: Select all
   class dgHumanoid : public ndModel
   {
   public:
      dgHumanoid();
      virtual ~dgHumanoid();
      //
                ...
   private:
      dgentity* m_head;
   };

    dgHumanoid::dgHumanoid()
    : ndModel()
    , m_head(nullptr)
    {
        m_head = OX3D::m_context->AddNTCube(glm::vec3(1.25f, 1.25f, 1.25f));
        m_head->m_name = "head";
        m_head->m_diffuse = glm::vec4(1.0f, 1.0f, 1.0f, 1.0f);
        m_head->m_trans->SetPosition(0.0f, 6.0f, -6.0f);
        m_head->SetMass(20.0f);
        m_head->InitNewton();
    }

    dgHumanoid::~dgHumanoid()
    {
          if (m_head) {
              delete m_head;
              m_head = nullptr;
          }
    }

void dgdemo05::InitCommandGL()
{
    if (!m_biped) {
        m_biped = new dgBipedalRobot::dgHumanoid();
        ndSharedPtr<ndModel> bipedPtr(m_biped);
        OX3D::m_context->m_newton->AddModel(bipedPtr);
    }
}

void dgdemo05::HaltCommandGL()
{
    if (m_biped) {
        OX3D::m_context->m_newton->RemoveModel(m_biped);
        m_biped = nullptr;
    }
}
Last edited by Dave Gravel on Sat Apr 08, 2023 4:15 am, edited 2 times in total.
You search a nice physics solution, if you can read this message you're at the good place :wink:
OrionX3D Projects & Demos:
https://orionx3d.sytes.net
https://www.facebook.com/dave.gravel1
https://www.youtube.com/user/EvadLevarg/videos
User avatar
Dave Gravel
 
Posts: 800
Joined: Sat Apr 01, 2006 9:31 pm
Location: Quebec in Canada.

Re: Smart Pointer confusion

Postby JoeJ » Sat Apr 08, 2023 2:24 am

Dave Gravel wrote:The object that I created inside the class is automatically deleted when I reset the scene and remove the model.

Ah yes, thought about that just after waking up. So i don't need to use the swapping trick for each body of a model. I'll comment it out in my code above.
For the reset button i still need it, because i don't reset the whole scene just this single model.
But that's just once for the model and no practical example anyway.
So i'm happy now. :)

Btw, i use this swapping trick usually with std::vectors. Depending on implementation, neither clear() nor resize(0) and shrink_to_fit() guarantee the reserved memory is freed. Thus the swapping trick with an empty vector is the only way to ensure this, they say. So using that trick maybe is common practice and does not feel toe hacky.

But i do see the costs: One more level of indirection causing pointer chasing and cache misses at high granularity,

Saying this isn't fair because the wrapped pointer does not need to be chased.
So i need to explain:
Before the shared pointer was introduced, i have got it to work using arrays of bodies and joints per model. It required some hacks, e.g. adding a default constructor to the base joint.
The advantage would be that cost of memory management is only once per model, not for all its bodies and joints. They also would end up serially in memory, eliminating one level of indirection when iterating them. So both the generation when streaming open world, and the processing itself would be faster.
I had not achieved this for shapes and nodes yet, but i was optimistic to make this work as well to some degree.
But after introduction of shared pointers i had to undo all those changes, falling back to the default way of managing each object individually.

Well, maybe i'll try this again sometime, but it's like fighting windmills or 'OOP dictatorship' :mrgreen:
I better postpone this as far back as possible.

Thanks guys!
User avatar
JoeJ
 
Posts: 1453
Joined: Tue Dec 21, 2010 6:18 pm

Re: Smart Pointer confusion

Postby Julio Jerez » Mon Apr 10, 2023 2:38 pm

I am saying this.
Joe, I am not really a fan of share point either, but I am not an enemy.
I thought a lot before going to the share pointer solution. But concluded that is one of the best solutions. here is the problem I try to solve.
All previous version of Newton, the Interface was C.

since C does not have a way to generate an instance of an object, the solution was a set of
Create/Destroy functions.
As the engine grew in size and complexity, it becomes obvious that the increase if these housekeeping functions become very hard to manage. That's one of the motivations to move to C++.
but with the move to C++, there is the problem of who has the owner ship of an object.

In before 4.00 the owner of an object was the world, but now is C++ that's not longer the case since an application can now create instances and refence to objects.

for a while though that I could keep the word been the owner. That broke down in many, many time and I keep fixing them by adding function like
Remove, Destroy, Release, and so on, each time making the logic more and more convoluted and the problem was not fixed 100%.

take for example a dModel. It can have reference to Bodies and Joints.
this brings the problem that the order with creation and destruction,
because when a body is reference by the world and Vehicle, who owns it.
I managed to get that working for a while until it across the citation that a body can be referenced by multiple models. and it keep getting more and more difficult to manage.

I realized that neither the world or the model, or any other class should claim owner ship of another object, they should have references and that's the problem that share pointers solve.

yes there are other way to address this. and Shere pointe also have some serious problem, like the possibility of circular references, and the also use more memory.

but that do render a service, than when solving it in different ways, it usually end up reinvesting a Share pointer paradigm.

One misconception about share pointer is that they are slower, that's not true. or at least epistemically not true.
a share pointe is a instance that contain a pointer to an object. so the code reduces to an offset, but not to a double indirection.

anyway, Joe, I suggest that you ready some of the literature about share pointers. Wikipedia provide some good explanation and justifications. not just for newton, but for your own programming.
https://en.wikipedia.org/wiki/Smart_pointer

and them you will see that although that are not perfect, they do render a service when use properly.
and again, yest that could be really bad what use incorrectly. we just do not do that whenever we can or at least no intentionally.
Julio Jerez
Moderator
Moderator
 
Posts: 12249
Joined: Sun Sep 14, 2003 2:18 pm
Location: Los Angeles

Re: Smart Pointer confusion

Postby Julio Jerez » Mon Apr 10, 2023 2:50 pm

btw, the reason I decided not to use the STD version.
was that C++ 11 introduced them, and I had a Template class that wrapped them, but then

with VS 2019 I start to get lot ow warning of deprecated functions.
so I decided that since after C++ 11 the language is going over ton of crazy modification,
that it was not worse the effort to keep adding fixed to the wrapper.

This way the engine is still C++ 11 compatible. although I nodes start to see some pedate warning with VS 2022 again.
Julio Jerez
Moderator
Moderator
 
Posts: 12249
Joined: Sun Sep 14, 2003 2:18 pm
Location: Los Angeles

Re: Smart Pointer confusion

Postby JoeJ » Tue Apr 11, 2023 3:42 am

Julio Jerez wrote:there is the problem of who has the owner ship of an object.

That's easy: The application. So in your case, you would manage ownership in the sandbox, but not in the engine.
I just realize, i would prefer to call it library instead engine, as the engine term more likely associates with higher level concepts such as managing ownership.
But since there is no general solution to such problems they should not be part of a library meant to implement lower level functionality such as physics simulation.
This becomes even more true because the application, e.g. a game, basically must implement solutions for ownership management anyway. So we most likely end up with multiple implementations addressing the same problems, increasing complexity and difficulty to the programmer, who now has to know and keep all the implementations in mind.

In other words, smart pointers, but memory on static data should be optional components of the library at a higher levels, which the users then can adopt or not. And additionally, you do not need to implement them at all, offloading the responsibility to the users.
I see that's not in line with the OOP paradigm, and it's also not how any physics engine i ever saw works. They all manage their objects internally. But imo they shouldn't.
I also see you need memory management for your internal data anyway, so it's ofc. an obvious choice to make no difference between static and dynamic data, handling all with the same management and enforcing its adoption. But imo that's not ideal.

To be clear, what i mean with static data is, for example:
A model, consisting of shapes, bodies and joints, plus the nodes remaining static to that model, e.g. graphs or trees to form a ragdoll skeleton or spatial acceleration structure.
Eventually, we want to have all this in one block of memory. Then we can stream this block from disk quickly, and adding it to the simulation means only to connect pointers.
The memory manager then only has to manage one block, not dozens of object. We always remove or add it as a whole.
It will not always work this way. E.g. eventually we can not effort to store BVH on disk, and have to build it on load. Which makes this entire ideal block idea optional. It should be possible, but not enforced as a concept for everything.

The dynamic data would be contact constraints, top level acceleration structure, and maybe some stuff.
If you do need (or benefit from) shared pointers for that, i neither complain nor notice. And ofc. you need to manage the memory dynamically, without much options to make assumptions on something like 'ideal blocks'.
So exposing this system to be used also for the static stuff is fine and makes sense, but it should be optional on enforced.

One rule i personally try to follow with this 'static / dynamic' context in mind, is that potentially static objects must have a default constructor. Otherwise we can't generate the data without a need to construct objects, while what we actually want is just to copy the data. That's why i missed it in the base joint.
Another similar issue was when i hacked the static geometry to extend adjacency to neighboring objects. Functionality was available only by calling constructors, or it was private. That's the things i call OOP dictatorship. When things like abstraction meant to make things easier become actual obstacles.

and them you will see that although that are not perfect, they do render a service when use properly.

Yeah, it's like with most patterns or concepts. They try to make it easier using automation and abstraction. And it works for 90% of cases. But the remaining 10% become even more difficult and error prone. So the benefits and losses cancel each other out, and all that's left is the additional need to know the pattern or concept. :)

That's how it looks to me at least. An amateur who never did read any programming books, so i do not even know if i have used above fancy terms correctly. :mrgreen:
But maybe my 2 cents still have some value regardless.
User avatar
JoeJ
 
Posts: 1453
Joined: Tue Dec 21, 2010 6:18 pm


Return to General Discussion

Who is online

Users browsing this forum: No registered users and 55 guests