Singleton synchonization improvements

[GoogleCodeExporter]

there is a half dozen or so singletons in Cocos2d, that are frequently
called.  Currently, every call to a shared instance requires a
synchronization cost.  This could be changed to use 'double checked
locking', where only the first call is penalized by synchronization overhead.

attached is a patch to make these changes to trunk.

Original issue reported on code.google.com by kean....@gmail.com on 10 Dec 2009 at 2:01

Attachments:

• cocos2d_double_checked_locking_improvements.patch

[GoogleCodeExporter]

Original comment by ricardoq...@gmail.com on 10 Dec 2009 at 2:20

• Added labels: Type-Enhancement
• Removed labels: Type-Defect

[GoogleCodeExporter]

Can you please provide some support for the assertion that you can actually 
safely do
this:

if (!sharedScheduler) {

Before any lock/synchronization?

Please read the following carefully before implementing any changes:
"C++ and the Perils of Double-Checked Locking"
http://www.aristeia.com/Papers/DDJ_Jul_Aug_2004_revised.pdf

In summary this is the recommendation:
(Quoted from the source above, paraphrased code into objective-c style for 
clarity)

If you (or your clients) are concerned about the cost of locking a 
synchronization
object every time sharedInstance is called, you can advise clients to minimize 
such
calls by caching the pointer that instance returns. For example, suggest that 
instead
of writing code like this,
[[YourSingleton sharedInstance] doSomething];
[[YourSingleton sharedInstance] doSomethingElse];

clients do things this way:
YourSingleton* const instance = [YourSingleton sharedInstance];
[instance doSomething];
[instance doSomethingElse];

One interesting way to apply this idea is to encourage clients to make a single 
call
to sharedInstance at the beginning of each thread that needs access to the 
singleton
object, caching the returned pointer in thread-local storage. Code employing 
this
technique thus pays for only a single lock access per thread.
Before recommending caching of the result of calling instance, it’s generally 
a good
idea to verify that this really leads to a significant performance gain. Use a 
lock
from a threading library to ensure thread-safe Singleton initialization, then do
timing studies to see if the cost is truly something worth worrying about.

Third, avoid using a lazily-initialized Singleton unless you really need it. The
classic Singleton implementation is based on not initializing a resource until 
that
resource is requested. An alternative is to use eager initialization instead, 
i.e.,
to initialize a resource at the beginning of the program run. Because 
multithreaded
programs typically start running as a single thread, this approach can push some
object initializations into the single-threaded startup portion of the code, 
thus
eliminating the need to worry about threading during the initialization. In many
cases, initializing a singleton resource during single-threaded program startup
(e.g., prior to executing main) is the simplest way to offer fast, thread-safe
singleton access.

(end of quote)

You may also be interested in reading this:
http://avitzur.hax.com/2006/11/efficient_thread_safe_static_i.html

and this:
http://googlemac.blogspot.com/2006/10/synchronized-swimming.html

Original comment by myBuddyCJ@gmail.com on 11 Dec 2009 at 11:55

[GoogleCodeExporter]

Double checked locking can be done correctly, if memory barriers are used in the
right locations.

See http://ridiculousfish.com/blog/archives/2007/02/17/barrier/
It's a great read, very detailed.

You are correct, that the first check of "if (!sharedScheduler)"
is theoretically not safe without another memory read barrier, since it's 
possible to
reorder the read of the field.  BUT, there is a data dependency on that read, 
and all
common processors (except the crazy DEC Alpha) will not reorder the read.  So, 
the
patch is safe for Arm processors, along with any future Arm compatible chips 
Apple
could ever ship.

So the change is perfectly safe, but I suppose it could use a comment 
explaining why
an additional read barrier is not needed.

Original comment by kean....@gmail.com on 14 Dec 2009 at 1:24

[GoogleCodeExporter]

since cocos2d is not thread safe, I've removed the @syncronized thing.

resolution: sort of fixed.

Original comment by ricardoq...@gmail.com on 28 Jan 2010 at 11:25

• Changed state: Fixed

[GoogleCodeExporter]

Original comment by ricardoq...@gmail.com on 28 Jan 2010 at 11:25

• Added labels: Milestone-Release0.9

[GoogleCodeExporter]

Original comment by ricardoq...@gmail.com on 8 Feb 2010 at 4:21

• Added labels: Milestone-Release0.99.0

[GoogleCodeExporter]

This issue was closed by revision r1776.

Original comment by ricardoq...@gmail.com on 8 Feb 2010 at 6:35

[GoogleCodeExporter]

I don't see why Cocos2D should not be threadsafe. There are already multiple 
threads
used for async loading (yes NSOperations use threads too).

I propose that you re-introduce the locking for singleton initialization but 
with one
welcome performance improvement... Swizzling of the method to a non-locking 
version
after the first run. Check out this writeup on the basic concept:
http://googlemac.blogspot.com/2006/11/synchronized-swimming-part-2.html

And here is an easy macro that I made to apply this approach to a singleton 
class:
http://github.com/wiseganesha/Objective-C-Optimized-Singleton

I've applied this change to my version of Cocos2D and it's working great. If 
you want
it included in the official distro, just post a comment here and I'll take the 
time
to make the correct patches against the latest trunk.

Original comment by myBuddyCJ@gmail.com on 28 Feb 2010 at 10:06

[GoogleCodeExporter]

yes, it would be nice to add threadsafe to cocos2d.
but the @syncronize doesn't add threadsafe. There are many many places that 
needs to 
be fixed. Also, take into account that although it is possible to use OpenGL 
from 
another thread, it is just APITA.

BTW, I like your macro.

Original comment by ricardoq...@gmail.com on 28 Feb 2010 at 2:09

[GoogleCodeExporter]

Sorry, I am new to development, what are the implications if cocos2d is not 
thread safe?

Original comment by ryanchan...@gmail.com on 24 Feb 2011 at 3:51