> On Apr 26, 2017, at 3:07 PM, Jonathan Schleifer <[email protected]> > wrote: > >> Unfortunately, I don’t remember the specifics anymore. This happened when I >> was converting Pacifist from MRC (Medieval Reference Counting ;-) ) to the >> newer Objective-C 2.0 with the @property syntax and ARC, which was years ago >> (nowadays, I’m converting it to Swift). I do think it had something to do >> with my model objects representing a file or folder inside a package; when >> opening the OS X installers, there end up being a *lot* of these, and some >> operations—I don’t remember exactly what—went from “Hmm, this is taking a >> little while” to “Hang on, let me go make a cup of coffee” after the >> conversion. I can’t remember if this was due to the spinlock, the >> autorelease pool getting slammed, or both, but going back and making all the >> @properties nonatomic solved it. > > In general, if nonatomic really made the difference, this sounds like > something was hammering the property way more than it should - so I guess the > actual bug would be elsewhere, but the atomic just made it more visible.
How can you say this without knowing the use case? Anyway, let me assure you that accessing the properties of the model objects was essential to performing the task the program needed to do. >>> 0000000100000e42 callq 0x100000ec8 ## symbol stub for: >>> _objc_retainAutoreleasedReturnValue > > This is where the problem occurs: It's calling > objc_retainAutoreleasedReturnValue, when it should call objc_retain. What? No it shouldn’t. objc_retainAutoreleasedReturnValue is the correct call to use here. If the method returning the value were written using ARC, it would cause it to skip the retain/autorelease dance and just return the +1 refcounted object. And if the method were MRC (which it is, in this case), it just does the same thing as objc_retain. Eitherway, you end up with a +1 refcounted object of which you take ownership. It’s really quite nifty. You can read about it here: https://clang.llvm.org/docs/AutomaticReferenceCounting.html#arc-runtime-objc-retainautoreleasedreturnvalue >> As you can see, objc_retainAutoreleasedReturnValue is called as soon as we >> receive the result of valueForKey:, and is not released until Baz() exits, >> thus ensuring that ‘foo' will remain valid until we are done with it. > > Except that what was returned never was retained and autoreleased! > valueForKey: just calls the property. If it does not retain and autorelease, > what it returns is not retained and autoreleased. The following code > demonstrates this easily: > >> #import <Foundation/Foundation.h> >> >> @interface Foo: NSObject >> @property (nonatomic, retain) Foo *test; >> @end >> >> @implementation Foo >> - (id)retain >> { >> NSLog(@"retain"); >> return [super retain]; >> } >> >> - (id)autorelease >> { >> NSLog(@"autorelease"); >> return [super autorelease]; >> } >> @end >> >> int main() >> { >> Foo *f = [Foo new]; >> >> [f setTest: [Foo new]]; >> [f valueForKey: @"test"]; >> >> return 0; >> } > > (compile as MRC, as ARC does not allow overriding retain / autorelease) > > This outputs retain once instead of twice and no autorelease. Using ARC does > not change how valueForKey: behaves. > => You call objc_retainAutoreleasedReturnValue on something that is not > retained or autoreleased. > => You can end up with a dangling pointer. We’re talking about ARC, not MRC. In ARC, the value will be retained after it’s returned by valueForKey:. Well, maybe not here, since you didn’t actually store it in a variable. But typically, yes. In ARC, you can’t override retain, but you can use Instruments to inspect all the places where the object was retained and/or released. With this code: > 1 #import <Foundation/Foundation.h> > 2 > 3 @interface Foo: NSObject > 4 @property (nonatomic, retain) Foo *test; > 5 @end > 6 > 7 @implementation Foo > 8 @end > 9 > 10 int main() { > 11 Foo *f = [Foo new]; > 12 > 13 [f setTest: [Foo new]]; > 14 Foo *test = [f valueForKey: @"test"]; > 15 > 16 [test self]; > 17 > 18 return 0; > 19 } Instruments reports that the second Foo (the one we stored in ‘test’) is: 0 Malloc +1 1 00:00.599.705 foo main (main.m line 13: [f setTest: [Foo new]];) 1 Retain +1 2 00:00.599.743 libdyld.dylib start (main.m line 13: [f setTest: [Foo new]];) 2 Release -1 1 00:00.599.747 foo main (main.m line 13: [f setTest: [Foo new]];) 3 Retain +1 2 00:00.599.953 foo main (main.m line 14: Foo *test = [f valueForKey: @"test"];) 4 Release -1 1 00:00.599.963 foo main (main.m line 19: }) 5 Release -1 0 00:00.599.974 foo main (main.m line 19: }) So: it gets allocated when we ask for a new object, and then there’s a balanced retain/release that we don’t have to care about. After it’s returned to us by valueForKey:, it’s retained. When it falls out of scope and we’re done with it, it gets released. Simple, easy, reliable—ARC in a nutshell. >> To my mind, in an ARC world, autorelease is nothing but an unneeded >> performance hit > > This is not true. There's a neat little hack which makes sure the object > never is autoreleased when using ARC: It looks at the callsite to see if > objc_retainAutoreleasedReturnValue is called, and if so never puts it into > the autorelease pool. Wait, what? I thought you were just arguing that objc_retainAutoreleasedReturnValue should not be used? Anyway, it’s a neat hack, but it’s imperfect. I know this because I still get objects piling up in autorelease pools, even with ARC on. I think most of it is from objects created inside the frameworks, which are largely MRC for legacy reasons, but it’s not only that. Look at this: > #import <Foundation/Foundation.h> > > @interface Foo: NSObject > @property (atomic, retain) Foo *test; > @end > > @implementation Foo > @end > > int main() { > Foo *f = [Foo new]; > > [f setTest: [Foo new]]; > Foo *test = f.test; > > [test self]; > > return 0; > } Running that in Instruments, I get: Malloc/Release/Retain/Autorelease (6) +1 00:00.538.314 foo main 0 Malloc +1 1 00:00.538.314 foo main 2 Release -1 1 00:00.538.328 foo main 3 Retain +1 2 00:00.538.331 foo main 4 Autorelease 00:00.538.334 libdyld.dylib start <- - - ***wat*** 5 Retain +1 3 00:00.538.342 foo main 6 Release -1 2 00:00.538.344 foo main Retain/Release (2) 00:00.538.325 libdyld.dylib start 1 Retain +1 2 00:00.538.325 libdyld.dylib start 7 Release -1 1 00:00.538.360 foo main Anyway, the fact that the compiler and runtime try (however imperfectly) to eliminate usage of autorelease wherever possible doesn’t contradict my point that autorelease is unnecessary and wasteful; rather reinforces it, actually. >> which only gains relevance if there is a chance that legacy MRC code might >> try to access the property > > MRC is not legacy. Some people just prefer it. And in that case, those people should understand that they’re not guaranteed that an object returned from a property getter will hang around forever, and should manually retain it. >> and in a typical modern project, the only MRC code I can imagine being >> involved would be in the Cocoa frameworks themselves. And if framework code >> is grabbing a value from code from God knows where that might do God knows >> what, not copying or retaining the value, and then, before it’s done, >> calling a callback that might change the unretained value that it >> grabbed—well, I’d argue that’s a bug in the framework. > > So you'd argue to always add an extra retain + autorelease in valueForKey:? > This would over-retain in the atomic case and retain in the nonatomic case. Nope, just that the people out there who are still writing Objective-C in MRC mode (both of you ;-) ) should code defensively. Retain the stuff you get and release it later, instead of relying on valueForKey: or the property getter to do it for you. Charles _______________________________________________ Do not post admin requests to the list. They will be ignored. Objc-language mailing list ([email protected]) Help/Unsubscribe/Update your Subscription: https://lists.apple.com/mailman/options/objc-language/archive%40mail-archive.com This email sent to [email protected]
