Hi Jonathan

nice to hear from you again (we met at Fosdem a few years ago, not sure you remember me; I do remember you). ;-)

You are right that the GNU runtime does [[x retain] autorelease] before 
returning the value for an object, nonatomic synthesized property .  The reason 
everyone but Apple does it, I suppose, it is because everyone felt it is a bug 
in the Apple implementation. ;-)

The additional retain/autorelease should make it safer (but slower) than the Apple implementation; I don't think it should cause it to behave differently (other than prevent the object from being released until the next autorelease pool is emptied).  Do you have a testcase where an actual difference in 
behaviour (other than the object being safe to use for longer, and the 
implementation with autorelease/retain being obviously slower) can be seen ?

I guess you're concerned about performance ?

Do you have any evidence that Apple did that on purpose and it's not a bug ?  
If you have any such evidence [ie, they won't "fix" it at the next release and 
we'll suddenly be the "broken" ones ;-)], we should certainly change it to 
behave in the same way.

Maybe we should change it to behave in the same way anyway ;-)

Thanks

PS: Your contribution to GCC's libobjc would be much appreciated.  The existing 
implementation of properties accessors is fairly finished though --

http://gcc.gnu.org/viewcvs/trunk/libobjc/accessors.m?revision=165903&view=markup

It would be a one-liner to make the change if we want to.

Hi Nicola,

yes, I do remember our talk at FOSDEM and I plan to attend it next year again, since I unfortunately could not attend it this year.

Well, I guess it's not really like this because everyone felt this is better, I guess it is because Cocotron was the first to implement it (IIRC), then GNUstep looked how they solved it and now GCC looked how GNUstep solved it ;).

I think it's not a bug, but intended, as not only the Apple runtime handles it like that, but the doc also says it's "just a return". So the documented behaviour and the real behaviour are in sync, I guess the name "nonatomic" was just a bad choice and the real idea was to have a way to create a lightweight accessor which is mostly used internally. There are several hints that nonatomic is thought for internal use in the formulations used by Apple. Plus, if you care that much about performance that the spinlock is too much for you, then autoreleasing would definitely also be a problem for you ;).

I mostly use nonatomic in some special cases like exceptions where the object which caused the exception should not be retained and autoreleased to prevent exception-loops (for example, if autorelease caused the exception to be thrown, that'd be a problem here).

In the code you linked, is that objc_mutex_t a spinlock? If not, this might be performance problem.

Speaking of performance and contributing: I wrote my own runtime some time ago. The lookup is twice as fast as in libobjc in my tests (with ASM enabled even more). Maybe we could work on merging that into libobjc on next FOSDEM? I mostly wrote my runtime because there weren't any changes in the GNU runtime for a long time and I was happy to see that things finally changed with gcc 4.6 :).

If you want to have a look: https://webkeks.org/hg/objfw-rt
(Hint: Thread-safety is still missing, though the data structure for the lookup only needs a write-lock and no read-lock and can still be read while it is written and it's also missing exceptions (copying the exception.c from GNU libobjc works))

I think it would be great to also work with the guys from Clang. I think it would only make sense to have the same runtime on all non-Apple systems, regardless of the compiler.

Author: nicola
Date: Sun Dec 19 19:10:26 2010
New Revision: 168070

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=168070
Log:
In libobjc/:
2010-12-19  Nicola Pero  <nicola.pero@meta-innovation.com>

	PR libobjc/47012
	* accessors.m (objc_getProperty): If not atomic, do not
	retain/autorelease the returned value. (Problem reported by

Modified:
    trunk/libobjc/ChangeLog
    trunk/libobjc/accessors.m

Yes, I was actually thinking about this, and you're right - it makes sense not to use retain/autorelease! ;-)

'nonatomic' means that other threads are not involved.  Which also means that the programmer calling the accessor has full control of what happens (there aren't alternative flows of execution that may jump in); he should do the retain/autorelease himself if there is a risk that something he does while using the object returned may call the accessor setter and trigger a release of the object; else, he can get away without a retain/autorelease and get a good speedup.

And doing the same that Apple does is obviously helpful for portability.

So I made the change in subversion.

Thanks

PS: GCC is currently in bug-fixing mode only for 4.6 so we can't accept non-bug-fix changes, but as soon as it reopens, you're very welcome to contribute.  Faster method lookup sounds very exciting (and non-trivial).

Fixed in trunk (will be GCC 4.6).

Thanks

Well, I did not plan to get that included in 4.6.

If you are interested in optimizing the lookup: The lookup could be greatly improved if we also change the ABI, which is currently the limitation as the current ABI does not allow for coherent inline caching etc. This needs changes in the compiler, therefore I did not do that - maintaining a runtime AND patchset for two compilers really is too much work for a single person ;).

I'd love to work on that, will you visit FOSDEM next year? If so, we could meet up there and hack on that :). I'd really love to reduce the extremely huge speed difference between the GNU runtime and the Apple runtime. At the moment, dispatch is more than 5 times faster with GNU (at least that's what I measured some time ago on Leopard, I guess with Snow Leopard it might be even more).

Yes, I'll visit Fosdem - let's meet up there.

> In the code you linked, is that objc_mutex_t a spinlock? If not,
> this might be performance problem.

You are right, it's a normal lock.  Currently, libobjc just uses plain mutex locks everywhere.  We should improve on that in GCC 4.7 - there's lot that
can be done.

Thanks

Hm, a mutex can be a real problem there, I guess. Accessors are used all the time, having a kernel lock each time will be a giant slowdown, especially as most of the time, the lock is not held anyway and if it is, it's only held for a very short period of time.

I think this should be changed to spinlocks, even for 4.6. Should I create a bug report for that?

If the problem is that we don't know whether we have a spinlock implementation on the system and would need to patch configure.ac and this is too much change before the release, we could just use gcc's __sync_bool_compare_and_swap, spin 10 times and if we still could not acquire our spinlock give control to another process using yield().

Sure, go ahead and create a bug for it.  We can make the change for 4.6 if we make it safe.  By the way, changing configure.ac is not a problem, we can do that, particularly if it makes the change safer. ;-)

It could make the change safer - we could check that spinlocks are available, and use them if so; if not, we just fall back to using a standard mutex, which is slower but works on all platforms.

Thanks