Re: sync.m

[David Chisnall]

Thanks - I forgot to set a couple of flags in the new metaclass, which was 
confusing things later.

This is now working with libobjc2, so thanks for the report.  A few 
observations about the code in your sample:

Although this works, it is a really, really bad way of creating a singleton.  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.

In general, when a solution to a problem is both more developer effort and 
slower, it's not the correct one...

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

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];
        }
}

You should really use -copy, not -retain when assigning newName to name.  If 
newName is an NSString, this will be equivalent.  If it's an NSMutableString 
then this will create an immutable copy for you.

In the @interface, you don't need to declare methods that are declared in the 
superclass's @interface.  

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. 

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