Moveit: Favor std::make_shared over constructor(new for performance reasons.

[tylerjw]

When creating a std::shared_ptr with the constructor you make two allocations (one for the control block and one for the object). Using std::make_shared offers a performance advantage in that it does the allocation with one heap allocation.

The only downside to this is a std::week_ptr will keep the memory of the object from being freed because the control block is allocated in the same block of memory as the object and the week_ptr needs access to the control block.

One other side benefit of using make_shared is it offers some protection against memory leaks due to arbitrary evaluation of expressions creating the possibility of exceptions thrown allocating memory before a shared pointer is created to manage the memory and after it is created. There is a good explanation of this here: SO Post.

An example of this change would be changing pose_model_state_space_factory.cpp:89 from:
return ModelBasedStateSpacePtr(new PoseModelStateSpace(space_spec));
to:
return std::make_shared<PoseModelStateSpace>(space_spec);

[welcome[bot]]

Thanks for reporting an issue. We will have a look asap. If you can think of a fix, please consider providing it as a pull request.

[tylerjw]

Downside to this is it doesn't use the ObjectPtr syntax and therefore can't be converted to boost::shared_ptr via a change to the macro. Ignore and close this if the intent was to be able to change the shared_ptr used via that macro. The current state uses the macro at a very small performance cost.

[BryceStevenWilley]

IMO, using std::make_shared<...>(...) isn't too bad. If we want to keep the same NamePtr style, we could define macros like NameMakePtr(...) that can delegate to std::make_shared<Name>(...). Pretty low hanging fruit, but also pretty low priority.

[v4hn]

Personally, I don't think another change like boost:: -> std::make_shared is around the corner,
so I would prefer the more readable plain std::make_shared<...>().

Other maintainers might disagree.

Either way, I believe a pull-request is welcome (and quite straight forward).

[Bhavam]

Hi all , so after going through this issue following it up now turns out std::make_shared<...>() does offer some disadvantages such as pointer memory cannot be deallocated until control block is still alive , does this pose any problems as such (any opinion from maintainers ?) and if not ill be happy to open a PR for this issue making use of std::make_shared in appropriate places.

[tylerjw]

@Bhavam I'll happily review your change and champion getting it accepted by the maintainers if you make a PR for this.

[rhaschke]

@Bhavam, you are welcome to prepare a PR for this.
Note that it's easily possible to release a pointer in advance by calling reset().