* [Bug libobjc/47012] nonatomic Properties behave wrong
2010-12-19 12:10 [Bug libobjc/47012] New: nonatimic Properties behave wrong js-gcc at webkeks dot org
@ 2010-12-19 18:31 ` nicola at gcc dot gnu.org
2010-12-19 18:49 ` js-gcc at webkeks dot org
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: nicola at gcc dot gnu.org @ 2010-12-19 18:31 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47012
Nicola Pero <nicola at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |NEW
Last reconfirmed| |2010.12.19 18:31:43
CC| |nicola at gcc dot gnu.org
Ever Confirmed|0 |1
--- Comment #1 from Nicola Pero <nicola at gcc dot gnu.org> 2010-12-19 18:31:43 UTC ---
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug libobjc/47012] nonatomic Properties behave wrong
2010-12-19 12:10 [Bug libobjc/47012] New: nonatimic Properties behave wrong js-gcc at webkeks dot org
2010-12-19 18:31 ` [Bug libobjc/47012] nonatomic " nicola at gcc dot gnu.org
@ 2010-12-19 18:49 ` js-gcc at webkeks dot org
2010-12-19 19:10 ` nicola at gcc dot gnu.org
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: js-gcc at webkeks dot org @ 2010-12-19 18:49 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47012
--- Comment #2 from js-gcc at webkeks dot org <js-gcc at webkeks dot org> 2010-12-19 18:49:39 UTC ---
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug libobjc/47012] nonatomic Properties behave wrong
2010-12-19 12:10 [Bug libobjc/47012] New: nonatimic Properties behave wrong js-gcc at webkeks dot org
2010-12-19 18:31 ` [Bug libobjc/47012] nonatomic " nicola at gcc dot gnu.org
2010-12-19 18:49 ` js-gcc at webkeks dot org
@ 2010-12-19 19:10 ` nicola at gcc dot gnu.org
2010-12-19 19:12 ` nicola at gcc dot gnu.org
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: nicola at gcc dot gnu.org @ 2010-12-19 19:10 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47012
--- Comment #3 from Nicola Pero <nicola at gcc dot gnu.org> 2010-12-19 19:10:29 UTC ---
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug libobjc/47012] nonatomic Properties behave wrong
2010-12-19 12:10 [Bug libobjc/47012] New: nonatimic Properties behave wrong js-gcc at webkeks dot org
` (2 preceding siblings ...)
2010-12-19 19:10 ` nicola at gcc dot gnu.org
@ 2010-12-19 19:12 ` nicola at gcc dot gnu.org
2010-12-19 19:15 ` nicola at gcc dot gnu.org
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: nicola at gcc dot gnu.org @ 2010-12-19 19:12 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47012
--- Comment #4 from Nicola Pero <nicola at gcc dot gnu.org> 2010-12-19 19:12:33 UTC ---
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).
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug libobjc/47012] nonatomic Properties behave wrong
2010-12-19 12:10 [Bug libobjc/47012] New: nonatimic Properties behave wrong js-gcc at webkeks dot org
` (3 preceding siblings ...)
2010-12-19 19:12 ` nicola at gcc dot gnu.org
@ 2010-12-19 19:15 ` nicola at gcc dot gnu.org
2010-12-19 19:19 ` js-gcc at webkeks dot org
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: nicola at gcc dot gnu.org @ 2010-12-19 19:15 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47012
Nicola Pero <nicola at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution| |FIXED
--- Comment #5 from Nicola Pero <nicola at gcc dot gnu.org> 2010-12-19 19:14:49 UTC ---
Fixed in trunk (will be GCC 4.6).
Thanks
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug libobjc/47012] nonatomic Properties behave wrong
2010-12-19 12:10 [Bug libobjc/47012] New: nonatimic Properties behave wrong js-gcc at webkeks dot org
` (4 preceding siblings ...)
2010-12-19 19:15 ` nicola at gcc dot gnu.org
@ 2010-12-19 19:19 ` js-gcc at webkeks dot org
2010-12-21 10:56 ` nicola at gcc dot gnu.org
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: js-gcc at webkeks dot org @ 2010-12-19 19:19 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47012
--- Comment #6 from js-gcc at webkeks dot org <js-gcc at webkeks dot org> 2010-12-19 19:19:33 UTC ---
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).
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug libobjc/47012] nonatomic Properties behave wrong
2010-12-19 12:10 [Bug libobjc/47012] New: nonatimic Properties behave wrong js-gcc at webkeks dot org
` (5 preceding siblings ...)
2010-12-19 19:19 ` js-gcc at webkeks dot org
@ 2010-12-21 10:56 ` nicola at gcc dot gnu.org
2010-12-21 11:26 ` js-gcc at webkeks dot org
2010-12-21 11:39 ` nicola at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: nicola at gcc dot gnu.org @ 2010-12-21 10:56 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47012
--- Comment #7 from Nicola Pero <nicola at gcc dot gnu.org> 2010-12-21 10:56:33 UTC ---
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug libobjc/47012] nonatomic Properties behave wrong
2010-12-19 12:10 [Bug libobjc/47012] New: nonatimic Properties behave wrong js-gcc at webkeks dot org
` (6 preceding siblings ...)
2010-12-21 10:56 ` nicola at gcc dot gnu.org
@ 2010-12-21 11:26 ` js-gcc at webkeks dot org
2010-12-21 11:39 ` nicola at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: js-gcc at webkeks dot org @ 2010-12-21 11:26 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47012
--- Comment #8 from js-gcc at webkeks dot org <js-gcc at webkeks dot org> 2010-12-21 11:26:37 UTC ---
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().
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug libobjc/47012] nonatomic Properties behave wrong
2010-12-19 12:10 [Bug libobjc/47012] New: nonatimic Properties behave wrong js-gcc at webkeks dot org
` (7 preceding siblings ...)
2010-12-21 11:26 ` js-gcc at webkeks dot org
@ 2010-12-21 11:39 ` nicola at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: nicola at gcc dot gnu.org @ 2010-12-21 11:39 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47012
--- Comment #9 from Nicola Pero <nicola at gcc dot gnu.org> 2010-12-21 11:38:56 UTC ---
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
^ permalink raw reply [flat|nested] 10+ messages in thread