Lazy Loading of KVO Observed Properties
While in one of the labs at WWDC, I helped someone out with a deceptively tricky bit of code. The gist is this: how do you handle the case of a lazily-loaded property which has Key-Value Observer clients? It may not be clear why this is challenging, so let's take a quick look.First, by "lazy loading," I mean essentially "load on demand." If setting up a property is particularly expensive — such as loading a large file from disk — you may want to delay doing that work until the property is actually needed. The idea is to keep the application responsive and not waste cycles and memory on something that the user may not actually use during a given session of using your app.
In this case, the property we wanted to load lazily was an image. Initially, the code looked like this (note this is not garbage collected Objective-C 2.0 code):
- (NSImage*)myImage
{
if ( myImage == nil )
{
[self setMyImage:[self fetchImageFromDisk]];
}
return myImage;
}
- (void)setMyImage:(NSImage*)aValue
{
NSImage* oldMyImage = _myImage;
_myImage = [aValue retain];
[oldMyImage release];
}
This all looks reasonable, but there's a subtle issue here. If there's a KVO observer (which is used for Cocoa Bindings), the very act of using the setter will generate a call to the getter. The reason for this is that observers may want to know what the previous value was in addition to the new value — which is helpful for things like Undo.
If you run through the logic in your head, you might discover how this plays out in real life:
-myImage
-setMyImage
-myImage
-setMyImage
-myImage
-setMyImage
And so on into infinity. Remember, each time you call -setImage, the KVO observers may first want to know what the existing value is. Because of the way this code is setup, the getter is called recursively, and the instance variable never gets an actual value set. It just stays at nil, so it keeps trying to rebuild the image and spirals into recursion.
So we need some catch in the code to realize that we're already trying to regenerate the image from a call higher in the stack, and just let the code pass right through. Here's the solution we came up with:
- (NSImage*)myImage
{
static isBeingSet = NO;
if ( myImage == nil && isBeingSet == NO )
{
isBeingSet = YES;
[self setMyImage:[self fetchImageFromDisk]];
isBeingSet = NO;
}
return myImage;
}
- (void)setMyImage:(NSImage*)aValue
{
NSImage* oldMyImage = _myImage;
_myImage = [aValue retain];
[oldMyImage release];
}
The setter is exactly the same. The only addition is a static BOOL variable that says we're already in the process of setting the value, so no need to do it again. In Objective-C (and C in general), a static variable keeps its value even across multiple calls to the same method. It's like the method itself has an instance variable.
The end result of this is that the first call to -myImage will generate a legitimate check on the value of myImage. Once we decide the image needs to be loaded, we set the isBeingSet static variable to "YES", so that when we call -setImage and KVO wants to fetch the existing value, we just return nil. After the value is set, we set isBeingSet back to "NO" and return the actual image.
You might wonder why we don't just set the instance variable directly in the getter. In other words:
- (NSImage*)myImage
{
if ( myImage == nil )
{
myImage = [[self fetchImageFromDisk] copy];
}
return myImage;
}
This is an inelegant solution because it breaks the Key-Value Observing contract. If you set instance variables directly, KVO has no way of knowing they've changed. Even though that would temporarily fix the recursion issue, we might run into other elusive bugs further down the road. In addition, it might be that the setter has some custom logic, such as setting a particular size before actually setting the value. By setting the instance variable directly, we'd be breaking that logic.
Keep in mind that none of this solves the broader issue of thread safety. If you have multiple threads accessing myImage, you would want to set up proper NSLocks or some other generalized thread-safe solution.
Lazy Loading of KVO Observed Properties
Posted Jun 16, 2007 — 33 comments below
Posted Jun 16, 2007 — 33 comments below
Joachim Bengtsson — Jun 16, 07 4381
Scott Stevenson — Jun 16, 07 4382
You're right, but it doesn't matter if all of this is being done in a single thread since only one getter is ever being called at a time. The example is simplistic by design. Here's an example for what you suggest (the setter should be the same as above):
@interface MyClass : NSObject { NSImage myImage; BOOL myImageIsBeingSet; } @end @implementation class - (id)init { if ( self = [super init] ) { myImageIsBeingSet = NO; } return self; } - (NSImage*)myImage { if ( myImage == nil && myImageIsBeingSet == NO ) { myImageIsBeingSet = YES; [self setMyImage:[self fetchImageFromDisk]]; myImageIsBeingSet = NO; } return myImage; } @end
Justin — Jun 16, 07 4383
- (NSImage*)myImage { if ( myImage == nil ) { [self willChangeValueForKey:@"myImage"]; myImage = [[self fetchImageFromDisk] copy]; [self didChangeValueForKey:@"myImage"]; } return myImage; }
Any code that acts on the image can be put into a third method used by the setter and getter.
Scott Stevenson — Jun 16, 07 4384
That triggers the same KVO notification as the setter, so you end up back in infinite recursion.
Harvey Swik — Jun 16, 07 4385
...huh?
Scott Stevenson — Jun 16, 07 4386
Fixed, thanks.
Robert Stainsby — Jun 17, 07 4390
Scott Stevenson — Jun 17, 07 4391
That's essentially the same thing as setting the instance variable directly, except it manipulates the data in the managed object storage. I believe you'd still need to use willChangeValueForKey/didChangeValueForKey in order to send KVO notifications, which would get us back into recursion.
ssp — Jun 17, 07 4392
I would have thought that the value of 'myImage' doesn't really change in the process of this lazy loading and thus observers shouldn't been notified.
Could you elaborate on the thoughts behind doing this and how the philosophy of KV-stuff works in your opinion?
(I find it hard to cram all of Apple into your anti-spam text field - not even the  symbol will work ;)
Vincent Verville — Jun 17, 07 4394
Note - I may have misunderstood your problem: Are you in fact looking for a way to avoid doing it this way?
-(NSImage*)image { if (image == nil) { // NSURL-style async handling of resources. // TTAppRez knows to ignores us if the request is (pending|ready). // will call [self RezResourceDidFinishLoading:] when ready. NSString* path = @"http://theocacao.com/images/leopard.png"; [TTAppRez loadResourceDataAt:path notifyingClient:self usingCache:YES]; } return image; } -(void)setImage:(NSImage*)newImage { NSImage* oldImage = image; image = [newImage retain]; [oldImage release]; } #pragma Resource Callbacks - (void)RezResourceDidFinishLoading:(TTAppRez *)sender { if ([self image] == nil) { NSImage* theImage = [[NSImage alloc] initWithData: [sender resourceDataUsingCache:YES]]; [self setImage:theImage]; [theImage release]; } }
Vincent Verville — Jun 17, 07 4395
The test on "image" in "RezResourceDidFinishLoading"
is to ignore the loaded resource if the image has already
been set to a non-nil value by something else.
Scott Guelich — Jun 17, 07 4396
But I also agree with ssp. If you hadn't been loading that value "lazily", no KVO notification would be sent because the image would have been initialized during the enclosing object's initialization -- before any KVO observers could have possibly started watching it. Sending a notification when it gets set later seems to be exposing an internal implementation detail.
Furthermore, if setMyImage: is public then your interface allowing another object to override the default image, but (if I'm correctly understanding what you're saying about setters implicitly calling getters) wouldn't this cause your object to unnecessarily load the default image from disk -- right before it's overwritten with the desired image?
Scott Guelich — Jun 17, 07 4397
Furthermore, if setMyImage: is public then your interface assumedly allows another object to override the default image...
Scott Stevenson — Jun 17, 07 4399
When the parent object is initialized, the value is nil. When the value is the lazily loaded, it becomes non-nil. So yes, the value is changing and I personally think KVO should know about it because otherwise you could end up with empty views and other confusing behavior.
(I find it hard to cram all of Apple into your anti-spam text field - not even the symbol will work ;)
"Apple" is all the input box is looking for. :)
@Scott Guelich: If you hadn't been loading that value "lazily", no KVO notification would be sent because the image would have been initialized during the enclosing object's initialization -- before any KVO observers could have possibly started watching it.
In my opinion, you should always try to honor the KVO contract, even if there's no obvious need to do so. It keeps things more predictable, particularly as the application changes. Consider this:
MyClass* myObject = [[MyClass alloc] init]; NSImage* currentImage = [myObject myImage]; [myObject setMyImage:nil]; currentImage = [myObject myImage];
A couple of things could happen here. If the data is always the same when loaded from disk, then this will work fine, but mostly by accident. If the image loaded from disk is not always the same, the new image will only be picked up if I've honored the KVO contract by sending out notifications even when I thought I didn't need to. If I don't honor the contract, then the bound objects will have incorrect data.
Sending a notification when it gets set later seems to be exposing an internal implementation detail.
No, it's being truthful about what's happening. The image object is changing, it's just that some might think the change is insignificant. Hiding that event is (in my opinion) is depending too much on a current behavior which could change later, causing hours of debugging.
Furthermore, if setMyImage: is public then your interface allowing another object to override the default image
I admit this is not a perfect example, it was just the most obvious one I could come up with.
Scott Stevenson — Jun 17, 07 4401
This is a good example, but slightly more involved than the sort of thing I have in mind.
Vincent Verville — Jun 17, 07 4402
Since KVO triggers the "get" accessor, you see that simply accessing the ivar will create an infinite loop unless:
a) you add extra state/logic to stop the loop. Or ...
b) you defer the "set" accessor to a third party who will do it after the "get". Or ...
c) Crazy untested idea: you defer the "set" accessor until after the "get" accessor using special ObjC messaging capabilities:
- (NSImage*)myImage { if ( myImage == nil ) { NSImage* theImage = [self fetchImageFromDisk]; [self performSelector: @selector(setMyImage) withObject: theImage afterDelay: 0]; } return myImage; }
d) Another crazy untested idea: you highjack the KVC mechanism to break the cycle by implementing more that one accessor for this ivar. For example, in addition to your special -myImage accessor that usually creates an infinite loop, implement a -getMyImage accessor that does the normal access. All the observers should pull the value returned by -getMyImage due to KVC precedence, and programmatically calling the -myImage accessor should trigger KVO without infinite loop.
e) others possibilities ? ...
Blain — Jun 18, 07 4405
MyClass* myObject = [[MyClass alloc] init]; NSImage* currentImage = [myObject myImage]; [myObject setMyImage:nil]; currentImage = [myObject myImage];
Wouldn't this mean that using nil as an indicator would be not good? I mean, if you're trying to release myObject's image and indicate that there is no image, setting to nil instead serves as a reset/reload, which probably isn't the intent. How about this, instead?
That way, if, for some reason, instead of loading the image, if the nib specifies an image, and it's later nilled out, the loadImage isn't inappropriately called if someone checks or is otherwise bound.
Also: is there any command that can set a bit/bool and return its previous value, atomically? That way, you don't need locks on loadedMyImage, and it'd be threadsafe.
Jean-Daniel Dupas — Jun 18, 07 4406
AFAK, it's going to trigger the notification after you really affect myImage ivar (notification is triggered by -didChange:), so the (myImage == nil) condition will be false on the second call.
Helge — Jun 18, 07 4407
OSAtomicTestAndSet()
Scott Stevenson — Jun 18, 07 4408
I admit I'm not entirely sure what you're referring to here, but the problem is that willChange generates a call to the getter, which causes the recursion.
Paul Thomas — Jun 18, 07 4410
You say that setting the ivar directly "breaks the Key-Value Observing contract", but you aren't changing the value of the property - just the 'cache' that you've called _myImage. Repeated calls to myImage or valueForKey:@"myImage" will return the same image, so what does it matter that a private ivar has changed?
Jens Alfke — Jun 19, 07 4411
The value is not nil. The instance variable is nil. But that is an internal detail of the implementation. (You might have filled in the ivar in the -init method, and the class would behave the same.)
The value of the property is the result of the accessor method; as far as the outside world is concerned, its value is completely unknown until it's asked for.
So in this case, the getter should not attempt to notify observers.
Jonathan — Jun 19, 07 4412
Keep in mind that with alignment a BOOL sandwiched between two words takes up a word of space, and space is at a premium with RAM being so (relatively) slow compared to cache. Group BOOLs in groups of 4 or 8 to get around that.
It would also be interesting to make a BlackHole class that tried to emulate the behavior of messaging to nil... though by virtue of not being standard it would be strictly limited to internal implementations and might not help that much, and NSInvocation generation is painfully slow in the present implementation.
Scott Stevenson — Jun 19, 07 4414
Joar and I were talking about this and came to the same conclusion. Started writing something up, but still considering the edge cases.
mmalc — Jun 19, 07 4415
Semantically, the value has not changed. It has simply been retrieved. As far as KVO is concerned, before it first asks for the value it is unknown. Therefore:
- (NSImage*)myImage { if ( myImage == nil ) { myImage = [[self fetchImageFromDisk] copy]; } return myImage; }
does not break the KVO contract.
The only unfortunate aspect of this implementation is that it does go against the Cocoa memory management mantra of using accessor methods everywhere. This is one of the cases though when you can break the rule knowingly based on experience.
mmalc
Scott Stevenson — Jun 19, 07 4419
I think for the most part this is okay, but what about when the value is set to nil at some point after it is loaded even (whether intentionally or not)? The observers may end up with incorrect data in some cases. Those cases may not be common, but they would be extremely difficult to debug.
I've seen so, so many bugs that were ultimately traced back to not honoring the KVO contract because the developer thought it wasn't important in that case. Obviously, your opinion holds weight, but personally, I think it's better to be truthful about when the value changes, even if you think no one is listening.
mmalc — Jun 19, 07 4423
The image should be set to nil using a KVO-compliant accessor, and it will therefore emit the appropriate change notification when it is actually changed.
If you really want to invoke a method to set the value in the get accessor, then implement a private set accessor (such as setPrimitiveMyImage:) that both the get and set accessors invoke...
I've seen so, so many bugs that were ultimately traced back to not honoring the KVO contract because the developer thought it wasn't important in that case.
Here it's important that the change notification not be posted -- the value hasn't changed.
(To seriously stretch an analogy, this is almost like measurements in quantum mechanics. By observing the experiment, you're affecting the outcome. If you haven't looked inside the box, you can only assume that the value is correct. As soon as you look, the value is correct and you'll be told if it ever changes again.)
Note also that this pattern is the same as that used by managed objects (see Managed Object Accessor Methods).
mmalc
MechaRonzilla — Jun 19, 07 4424
- The getter should call the private setter
- The public setter should call the private setter.
For the case of "what happens when setFoo: is called with a nil argument…", we get this sequence:
1) KVO calls the getter (during your object's willChange…)
2) Enter our public setter which calls our private setter which changes the value (what the new value is is unimportant)
3) KVO calls the getter (during your object's didChange…) - maybe the result is nil, maybe it's not. That's unimportant.
Now call your getter again. What's the value? The only thing that matters to KVO here is that it's the same as the value that was reported in step 3.
As an aside, I'd argue that the value in step 3 should be non-nil, unless fetchImageBlahBlah returned nil. The trick here is that we depend on fetchImageBlahBlah returning nil for eternity after that.
Now, the only two other places from which you should make calls to the private setter are
- init
- dealloc (assuming you've already removed observers)
Clear?
mark — May 25, 08 5917
This is what the Core Data Programming Guide has to say:
Faults and KVO Notifications
When Core Data faults in an object, key-value observing (KVO) change notifications (see Key-Value Observing Programming Guide) are sent for the object’s properties. If you are observing properties of an object that is turned into a fault and the fault is subsequently realized, you therefore receive change notifications for properties whose values have not in fact changed.
While the values are not changing semantically from your perspective, the literal bytes in memory are changing as the object is materialized. The key-value observing mechanism requires Core Data to issue the notification whenever the values change as considered from the perspective of pointer comparison. KVO needs these notifications to track changes across keypaths and dependent objects.
Ross Boucher — Jan 19, 09 6596
Why not stop automatic support of KVO? Override
+ (BOOL)automaticallyNotifiesObserversForKey:(NSString *)aKey
have it return no for this specific key, and then manually call
willChangeValueForKey:
and
didChangeValueForKey:
only when appropriate? It will end up being more lines of code, but it does seem like the more "correct" solution to the problem. Just a thought. Perhaps it isn't worth the extra code.
wackazong — Apr 21, 09 6708
I just tried that in a program of my own, and it actually stops ALL KVO from working, so other bindings could be affected.
BTW, isn't there a type declaration missing in the first solution? Shouldn't
static isBeingSet = NO;
be written as
static isBeingSet = NO;
wackazong — Apr 21, 09 6709
static BOOL isBeingSet = NO;
Blog design — Dec 29, 09 7033