Proposal for v3.0: shared_ptr vs. manual retain release

If we were to migrate to smart pointers, which API would you like:

a)

 // no typedefs
class Node
{
  void addChild( std::shared_ptr child);

  static std::shared_ptr create();
}

// Usage
std::shared_ptr n( Node::create() );

b)

// proposed by Andre Rudlaff

typedef std::shared_ptr Node;
class NodeType
{
  void addChild( Node child);

  static Node create();
}

// Usage
Node n( NodeType::create() );

c)

// proposed by Walzer

class Node
{
  typedef std::shared_ptr ptr;

  void addChild( Node::ptr child);

  static Node::ptr create();
}

// Usage
Node::ptr n( Node::create() );

Any particular suggestion? Any other option ? I would go with c) I guess.

Ricardo Quesada wrote:

Linus L wrote:
>
> Second that. 5x will be amortized to nonexistence in any decently complicated game, not to mention pass-by-ref can be used to get rid of almost any penalty in most situations. Please avoid explicit retain/release since it’s ugly to a point that it reduces productivity.
>
Interesting. I agree that passing smart pointers as reference will be faster. Carlo and Justin (teammates) did some experiments, and if you pass the smart pointers as reference the compiler won’t accept polymorphism.
[…]
>
Any idea how to solve it ? Is it possible at all ?
Thanks!

sorry, i didn’t get you…

the operator “->” of shared_ptr will return the raw pointer of the object. and the polymorphism is determined by the virtual table from the raw pointer.
When we cast a shared_ptr from shared_ptr by reference, nothing is involved the polymorphism procedure, its just named the object by reference.
so i don’t think this operation would modify the behavior of polymorphism.

in my opinion, the styles seems better.
: )

Ricardo Quesada wrote:

Linus L wrote:
>
> Second that. 5x will be amortized to nonexistence in any decently complicated game, not to mention pass-by-ref can be used to get rid of almost any penalty in most situations. Please avoid explicit retain/release since it’s ugly to a point that it reduces productivity.
>
Interesting. I agree that passing smart pointers as reference will be faster. Carlo and Justin (teammates) did some experiments, and if you pass the smart pointers as reference the compiler won’t accept polymorphism.
[…]
>
Any idea how to solve it ? Is it possible at all ?
Thanks!

  1. Pass by non-const ref is not implicitly allowed, because the callee may accidentally release the pointer and causes it’s caller to have an invalid pointer, which breaks the shared_ptr behavior(valid within its scope). In other words, if you ever find yourself need shared_ptr&(no automatic casting) than const shared_ptr& (automatic casting for polymorphism types), there is probably something unsafe going on.

  2. Pass by const ref is ok, since you can’t change its ref count inside the callee scope(most of the time), and that makes sure whatever’s passed in is still valid in its caller scope.
    But there are catches. Pass by const ref is not thread safe, nor is it completely safe in a single thread situation. please refer to http://stackoverflow.com/questions/327573/c-passing-references-to-boostshared-ptr/8844924#8844924 where the top answer composed a contrived example of when const-ref could fail.

In the context of cocos2dx, if you have a global container that holds all the pointers, and you have a removeChild method that takes a node by const shared_ptr&, and that ref propagates all the way up to being the ref to a member in the container, then when you try removing it first from the container(setting ref count to 0), then do some cleanup inside the node. This will likely cause a crash.
But I don’t see this being a stopper for adopting shared_ptr. My rule of thumb, pure functions, pass-by-const-ref. Potential side effects on the argument shared_ptrs? pass-by-value.
My bet is we won’t see much difference with or without const-refs.

Ricardo Quesada wrote:

If we were to migrate to smart pointers, which API would you like:
>
a)
[…]
>
b)
[…]
>
c)
[…]
>
Any particular suggestion? Any other option ? I would go with c) I guess.

Regarding the style, I would say B or C.
My reason for B is you almost never need to allocate a cocos2dx class on stack(Node, Sprite, Layer or whatever), so dropping the explicit pointer postfix shouldn’t be much of an issue since everything is an pointer. If you decided to go with style B, probably hiding constructors and forcing factory methods on all such objects?
C is just typical of how you would write a typedef shared_ptr type, though kinda verbose.

Prefer C.
B is also fine but it may be better to use the term Node and NodePtr instead

Linus L wrote:

  1. Pass by const ref is ok, since you can’t change its ref count inside the callee scope(most of the time), and that makes sure whatever’s passed in is still valid in its caller scope.
    But there are catches. Pass by const ref is not thread safe, nor is it completely safe in a single thread situation. please refer to http://stackoverflow.com/questions/327573/c-passing-references-to-boostshared-ptr/8844924#8844924 where the top answer composed a contrived example of when const-ref could fail.

Interesting stuff. From that link, I found this other link:

that is very useful as well.

I like c.

a. is fine too.

I think b. is less clear because if you know nothing about the codebase and just looked at some code using a Node, you’d think that it was an object not a pointer to an object.

I have just started looking into Cocos2d-x, but as a long time C*+ developer, I was immediately dismayed by the use of manual reference counting and object ownership via plain pointers. That’s considered bad practice in modern C*+ code and for good reason. However, it is somewhat heartening to see that this topic is already being discussed in the forums.

The use of smart pointers (and the whole concept of RAII[1]), in addition to automating reference counting, makes resource ownership policy explicit. If I see “std::shared_ptr foo;”, I immediately know that I am dealing with an owning (or “strong”) reference. If I see “Foo * foo;”, who knows? Do I need to release “foo”? Autorelease it? Do I own it in any sense?

Smart pointers also make resource management pretty much bullet proof even in the face of exceptions (an aspect that I have not seen discussed anywhere else in this thread).

Regarding “std::shared_ptr” adding overhead due to thread safety, well either we need the thread safety and we’re going to have to eat that overhead one way or another anyway, or we don’t need the thread safety, in which case we can use something other than “std::shared_ptr”. Writing a smart pointer class template is generally not very complex or difficult (“CCStrongPtr<>” anyone?). In fact, my first instinct upon seeing that Cocos2d-x used manual reference counting is that I would have to write one to take care of all of those reference counting calls for me.

Here are a couple good articles on the use of smart pointers:



[1] http://en.wikipedia.org/wiki/Resource_Acquisition_Is_Initialization

David,
Welcome to our land :wink: Manual reference counting is easier for Objective-C and Java programmers, and experienced C*+ programmers or teams can use smart pointers already.
Also migration to smart pointers in current state can be hard: cocos2d-x should compile on any step of this transition. Maybe process can be splitted to following steps:
# Find and remove cocoa data structures where it’s possible. Figure out where such replacement can cause perfomance regression because STL data structures have no garantued Copy-On-Write, and stop next steps until good solution will be accepted.
# Create cocos2d-specific SharedPointer and WeakPointer classes, designed for single-threaded use. WeakPointer should have optimizations: std::unordered_map instead of std::map to keep references and, maybe, unsigned _weakRefsCount member in cc::Node which increase perfomance in case when amount of weak refs is much lesser than amount of cc::Node instances.
# Create tweaks of CC_SYNTHESIZE macros group, WEAK andSTRONG respectively to simplify further steps.
# Move some classes to WeakPointer, good example is TouchHandler that, ideally, should not retain touch delegates.
# Remove autorelease and use only SharedPointer+WeakPointer. Ensure that all tests still work correctly, for Linux tests I can advice valgrind and -fsanitize=memory compiler option in recent clang/GCC
# Finally replace SharedPointer and WeakPointer with std::shared_ptr and std::weak_ptr using text replace. Ensure that all tests work again, write some multithreading tests, -fsanitize=thread
Unfortunately Visual Studio compiler lacks two very useful c*+ 2011 features: template type alias and non-static member initializers. Also, about std:: namespace - actually we can write using namespace std; inside namespace cc, so this using directive will be applied only for cc scope and doesn’t touch user code; not sure that it will be more useful than confusing.

[1] http://clang.llvm.org/docs/MemorySanitizer.html
[2] http://clang.llvm.org/docs/ThreadSanitizer.html

Sergey,

Manual reference counting is easier for Objective-C and Java programmers
Can you explain this? My impressions are that automatic reference counting is easier for new programmers since it is similar to garbage collection.

Also, I’ve noticed that references on StackOverflow say that building a smart pointer class seems very trivial, but can actually be difficult (http://stackoverflow.com/a/504432). With that in mind, why would building a smart pointer class help instead of moving directly to shared_ptr?

I can see how the C++11 features would be useful to clean up the code, but I don’t see how the lack of those features would restrict development.

Kevin H wrote:

Also, I’ve noticed that references on StackOverflow say that building a smart pointer class seems very trivial, but can actually be difficult (http://stackoverflow.com/a/504432). With that in mind, why would building a smart pointer class help instead of moving directly to shared_ptr?
At this moment any created object will be added to autorelease pool, which calls release() on end of each frame. If it also will be registered in shared_ptr (without removing it from autorelease pool), object will be deleted twice. This issue can be avoided by custom smart pointer that calls retain() and release() instead of taking ownership and calling delete on destruction. So custom smart pointer would help in transition.

I can see how the C++11 features would be useful to clean up the code, but I don’t see how the lack of those features would restrict development.
It doesn’t restrict, of course.

If the focus is to make the API C*+ friendly and remove objc idioms then I think the code snippets should look like this:
CCSequence *seq = new CCSequence({MoveBy(2, cpp(240, 0)), RotateBy(2, 540), ScaleTo(1, 0.1f), RemoveSelf()});
I’ve removed that pesky NULL as last parameter and eliminate the create method, which is another thing that doesn’t belong in C*+.

I’m against of shared_ptr in game engines due to memory fragmentation and I’m not a big fan of autorelase/release/acquire either. I believe that cocos2dx should internally manage the memory that is created and in that way keep the memory fragmentation to a minimum.
Example:
CCSequence *seq = CCMemoryPool::createSequence({MoveBy(2, cpp(240, 0)), RotateBy(2, 540), ScaleTo(1, 0.1f), RemoveSelf()});

CCSprite *s = CCMemoryPool::createSprite("a.png"); [...] CCMemoryPool::release(s);

Another way is to overload the new operator of the CCObject to use a memory pool.

Maybe we can find a middle ground: cocos2dx will not make any decision about the memory model and the users of the library can use shared_ptr if they prefer and the users who need a fine grained memory management can also use it.

can any one help ….how to install cocos2dx in ubuntu 13.04.

i think using a custom shared pointer could be the way to go. This way the reference count could still be in CCObject and this would allow to create a shared pointer by passing “this” as argument.
We could use a custom “create” method which returns a shared ptr and have protected constructors to prevent user from using raw pointers.
I thinks the best approach would be to use a notation such as
@
typedef CCSharedPtr NodePtr;

class Node{
public:
NodePtr create();

protected:
Node();
}

typedef CCSharedPtr NodePtr;
@

also to get rid of the Obj-c idioms we could start replace structs such as CCPoint with classes and add some more features.
Also some methods require arguments to be passed as allocated objects, these could be passed as const reference, for example:
CCSequencePtr seq = CCSequence::create( MoveBy(2, cpp(240, 0)), RotateBy(2, 540), ScaleTo(1, 0.1f), RemoveSelf());

Biplab De wrote:

can any one help ….how to install cocos2dx in ubuntu 13.04.
There are no good way to install it - cocos2dx designed for delivering with game bundle (with custom patches or without them). Just download cocos2dx source code and run “make all” in root directory. If you want to use IDE, you should select between Eclipse and QtCreator.
* For Eclipse, see [[How_to_run_HelloWorld_and_tests_on_linux|this wiki page]] and use any cocos2dx version;
* For QtCreator, see [[QtCreator_project_setup_on_Linux|this wiki page]] and clone git branch “develop” from official github repository: https://github.com/cocos2d/cocos2d-x/ (of course, you can download some release and backport .pri &.prf files from “develop” branch).

What’s the final decision on this?
I’d love to have some C++ smart pointers! :slight_smile:

They just work fine with the STL, no need of custom containers

Xavier Arias wrote:

What’s the final decision on this?
I’d love to have some C++ smart pointers! :slight_smile:
>
They just work fine with the STL, no need of custom containers

From the API point of view, we know that smart pointers are better than manual retain / release.
But before switching to whole API to smart pointers, we need to be sure that the performance won’t be affected.

And we still need to do more performance tests before making the final decision.

I’ll love to hear more about it.

Cocos2d-x v3.0 is going to be amazing!

Hi,I’m an ambassador of the retain/release model.
First of all, this model have the best backward compatibility to all cocos2dx projects.(There’s already so many cocos2dx projects, and keep growing, cocos2dx is the ground for all of them).
Second, Richard mentioned this model can not be leveraged with multi-thread,but actually, iOS SDK have done this, you can refer to the solution that iOS did:

  • make reference counting operation atomic (using atomic incremental/decremental ops),
  • Provide thread API in cocos2dx, and keep a root AutoReleasePool in each thread.
  • Implement RunLoop-like looper APIs, rebuild Scheduler and renderer upon the RunLoop APIs(Scheduler in main thread and renderer in render thread), then purge AutoReleasePool every loop.
  • For customized threads, it is user’s responsibilities to purge the thread-root AutoReleasePool by themselves, but AutoReleasePool can have purge-interval assertions in debug mode, to avoid misuse.
    And at last, for performance issue, there’s still enough space for release/retain mode to improve, like optimization on AutoReleasePool, like small objects allocator and so on.

In contrast, In my opinion share_ptr is easy to misuse, and not so familiar with all C++ developers, also it do not have the same capability as well as objc-style auto-release. but most of all , it changes the API too many.

What about making an allocator object, pretty much the ones that stl uses to separate the memory model from the construction/destruction of the objects itself.
And in that way people could use the one the like the most: shared_ptr or alloc/release model