Re: KVO and the observeValueForKeyPath bottleneck

FROM : Jakob Olesen
DATE : Tue Jul 18 16:06:32 2006

On 18/07/2006, at 3.00, Chris Kane wrote:

> You and Matt Neuburg and Jim Correia (privately) have all now 
> identified all the parts of the problem and the solution.


Well, not really. Note that removeObserver:forKeyPath: does not take 
a context argument. It just removes the last context added. This 
means it is not safe to remove yourself as an observer until dealloc 
(or didTurnIntoFault). You also need to trust that sub- and 
superclasses will do the same.

Otherwise you have to use a helper object.

> The solution is that the context pointer must be used to provide a 
> globally unique value that you can recognize in your class.  If you 
> recognize the value, this notification is for you, otherwise you 
> pass the method call up to super and return.  You have to use a 
> value that the other classes in the hierarchy won't (or can't) use.


This is of course the way to go, but in the process we have 
unanswered Matt's original question, "How are you guys doing the 
dispatch thing?"

...and it is really painful to waste a perfectly good void pointer on 
multiple personality resolution. :-)

How about this:

static SEL dispatch[10]; // make it big enough or die violently

static void* sel2ctx(SEL s)
{
    int i;
    for (i=0; i<sizeof(dispatch)/sizeof(dispatch[0]); i++) {
        if (!dispatch[i]) dispatch[i]=s;
        if (s==dispatch[i]) return dispatch+i;
    }
    abort(); // I told you...
}

- (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)
object change:(NSDictionary *)change context:(void *)context
{
    SEL *entry = (SEL*)context;
    if (entry>=dispatch && entry<dispatch+sizeof(dispatch)/sizeof
(dispatch[0])) {
        [self performSelector:*entry withObject:...];
    }
    else {
        [super observeValueForKeyPath:keyPath ofObject:object 
change:change context:context];
    }
}

fgrep -c @selector MyClass.m would be an upper bound on the necessary 
size of the dispatch table
Oh, and you would have to pre-fill the table if you are 
multithreaded. Call sel2ctx in +initialize or use an initializer list:

sed -n 's/^.*\(@selector([^)]*)\).*$/    \1,/p' MyClass.m | sort -u

This is not just a matter of performance, also convenience. You 
already have to keep your addObserver and removeObserver calls in 
sync. Adding custom dispatch code to observeValueForKeyPath makes it 
even harder to maintain.

Also, keyPath+object is not always so great for deciding what to do. 
Imagine observing the same keyPath on objects in two collections. You 
have to check the collections. Now imagine the same object present in 
both collections. You get two identical callbacks.

So what is the right thing to do? It depends.

For a heavy-weight class (few instances, lots of state) use a helper 
object.
If you need removeObserver outside dealloc/didTurnIntoFault, use a 
helper object.
If you need to observe self, use a helper object.
For a light-weight class (lots of instances, little state) use a 
unique context and custom dispatch if it can be kept simple, 
otherwise a dispatch table.
If you are working alone and writing a non-reusable NSObject 
subclass, just use a selector directly. (Don't complicate things 
until you have to).

> You can't include something *inside* the context, because you can't 
> safely look through the pointer at anything in particular, as the 
> pointer might not be pointing to the data structure you know 
> about.  You have to generate a unique pointer value and act based 
> on that.  An == comparison is dirt cheap if you can do it.


I have seen Windows code ask the virtual memory manager if an address 
is readable, then check for a magic value. Messy and not 100% safe, 
but possible.

> If you assume that @"" string constants are NOT globally uniqued in 
> a binary, but only to a given compilation unit, then 
> ThisClassUniqueObservationContext could be a constant string:
>
> static NSString * ThisClassUniqueObservationContext = @"MyContext";


CFSTR() does global uniqueing, at least in the open-sourced version, 
but there is also a mystery __builtin___CFStringMakeConstantString 
implementation.

> Note that if this were "const", that could thwart things, since 
> integer constants might also be uniqued and shared by the linker.


Really? I thought only C++ allowed that. C99? After all you are 
passing a pointer to your constant outside the compilation unit.

> But whatever way you dice it, "yuck".  Usually I've not wanted the 
> context pointer for anything and just wanted to use NULL ... but 
> for this potential problem.


I agree.

NSNotificationCenter causes less trouble by taking a selector instead 
of a void pointer. It still has the problem with removing observers, 
though. removeObserver:name:object: does not take a selector 
argument, so it is the same mess if you try to unregister before 
dealloc.