gnustep-dev
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: sync.m

From: icicle
Subject: Re: sync.m
Date: Sun, 28 Feb 2010 09:42:31 +0100
User-agent: Internet Messaging Program (IMP) H3 (4.0.4) 


Although this works, it is a really, really bad way of creating a singleton.


I copied it from the apple developer pages...


I strongly suggest that you create it in +initialize instead.

If you really must use @synchronized(), you should put a test
outside, like this:

if (nil == sharedInstance)
{
        @synchronized(self)
        {
                if (nil == NP_ENGINE_CORE)
                {
                        NP_ENGINE_CORE = [self new];
                }
        }
        return NP_ENGINE_CORE;
}

At least this way you will only have to do the (very expensive)
objc_sync_enter/exit thing once, or maybe twice.  If you implement it
with +initialize, however, you don't have to do it at all:

+ (void)initialize
{
        if ([NPEngineCore class] == self)
        {
                NP_ENGINE_CORE = [self new];
        }
}
+ (id)sharedInstance
{
        return NP_ENGINE_CORE;
}

The runtime will handle the locking for you, calling the +initialize
method precisely once, the first time the class is sent any message.



You can then guarantee that the singleton instance already exists by
the time the +sharedInstance method is called.  You could also then
eliminate the need for @sycnhronized() in +allocWithZone: because if
NP_ENGINE_CORE is still nil then you are being called from the
+initialize method.


I thought +initialize would be called on program startup for every
class. I want to be able to create my singleton on demand since it has
a rather large memory footprint. If that works with the method you
propose I'm fine with it.



Your +allocWithZone: method should really check that it's not a
subclass, before returning the singleton.


I don't want this class to be subclassed.



In -initWithName:, you should check that self != nil after [super
init].  See the SUPERINIT macro from EtoileFoundation.

In a singleton, it's a good idea to override -dealloc to not call
[super dealloc].  This will cause a warning with GCC, which you can
hide like this:

- (void)dealloc
{
        if (0)
        {
                [super dealloc];
        }
}


How would you deallocate your singleton then?



By the way, where did you get the spaces-inside-square-brackets style
from?  I don't think I've ever seen Objective-C written like that.


I didn't get it anywhere, that's my own :)

Thanks
TOM



David

On 27 Feb 2010, at 22:47, address@hidden wrote:


Hi!

I put together a testcase, this used to work before the switch
ObjectiveC2 framework. Lin 62 is the one causing the crash.

Thanks
TOM

Zitat von address@hidden:


Hi!

looks like it is not fixed :( Richard ported the code over to the
ObjectiveC2 framework immediately, but it still does not work. Now I
get "Uncaught exception NSInvalidArgumentException, reason:
(null)(class) does not recognize instance". As "instance" is the
class method which creates the singleton now something else seems to
go wrong.

Thanks
TOM

Zitat von David Chisnall <address@hidden>:


I've now fixed this case in libobjc2.  Unfortunately, someone
decided to 'helpfully' reindent the version of ObjectiveC2.framework
in GNUstep, which means that diffs from libobjc2 no longer cleanly
apply in ObjectiveC2 (nor to diffs against the original version in
Étoilé svn), so whoever did that gets to volunteer to back-port the
changes.

On 27 Feb 2010, at 17:40, Richard Frith-Macdonald wrote:


You are probably in a better position than anyone else to be aware
of precisely what parts of ObjectiveC-2 are most efficient or
inefficient, and how they compare to the traditional ways of doing
things.  Have you considered producing a paper describing those
differences?  If we had them quantified we would have a really good
guide for people to know when to use new features and when to avoid
them (and when it really doesn't matter).



Well, I did write a book that describes them...

@synchronized is basically impossible to implement efficiently.
It's a stupid feature added to make life easier for Java programmers
who are too lazy to think when they learn a new language.  There are
basically three ways you can do it:

1) Allocate a pthread_mutex_t with every object

Pros: Fast, simple
Cons: Wastes at least one word of memory for every object, including
the 99.9% that are never used as arguments to @synchronized().

2) Have a shadow data structure mapping objects to locks.

Pros: Doesn't waste much memory.
Cons: Extra locking on the shadow structure needed, extra code
needed to remove locks when they are no longer needed, overhead
performing the lookup.

3) Add a hidden class between the object and its real class which
stores the lock.

Pros: Relatively simple and non-invasive.
Cons: Needs an extra class structure for every locked object.

libobjc2 / ObjectiveC2.framework use option 3.  We use
objc_allocateClassPair(), which doesn't work for creating just a new
metaclass (although we fail slightly more gracefully than Apple's
version, which just returns a pointer to a random address).  I've
now added a special case to libobjc2 and a non-portable runtime
function for allocating just a new metaclass.

In all cases, this pattern will be much faster:

[nslock lock];
@try {
        // do stuff
} @finally {
        [nslock unlock];
}

It's also worth remembering that @synchronized uses a recursive
mutex, which, on most platforms, is slower than the non-recursive
form used by NSLock.  Unless you actually need the recursive
behaviour, don't use it.

@synchronized is a horrendous language feature, because it looks
just like the Java keyword, but has almost the exact opposite
performance characteristics.

David

-- Sent from my Difference Engine










_______________________________________________
Gnustep-dev mailing list
address@hidden
http://lists.gnu.org/mailman/listinfo/gnustep-dev





<synchronized.tar.gz>_______________________________________________
Gnustep-dev mailing list
address@hidden
http://lists.gnu.org/mailman/listinfo/gnustep-dev



-- Sent from my STANTEC-ZEBRA



_______________________________________________
Gnustep-dev mailing list
address@hidden
http://lists.gnu.org/mailman/listinfo/gnustep-dev
reply via email to
[Prev in Thread] Current Thread [Next in Thread]