Re: GSIMap

[David Chisnall]

On 2 Jun 2011, at 06:28, Richard Frith-Macdonald wrote:

> 
> On 1 Jun 2011, at 23:48, David Chisnall wrote:
> 
>> On 1 Jun 2011, at 23:20, Richard Frith-Macdonald wrote:
>> 
>>> 
>>> On 1 Jun 2011, at 19:30, David Chisnall wrote:
>>> 
>>>> Hi,
>>>> 
>>>> I'm trying to make NSHashTable / NSMapTable use the correct read / write 
>>>> barrier functions in GC mode, but I don't really understand the GSIMap 
>>>> code.  Does it define macros for reading / writing pointers anywhere?  In 
>>>> GC mode, we need to call the relevant read and write barrier functions for 
>>>> assigning pointer values, depending in the pointer functions:
>>> 
>>> NSHashTable and NSMapTable use both NSPointerFunctions and the old 
>>> callbacks (Apple added new classes for these objects, while retaining 
>>> backward compatibility with the old API)... see 
>>> NSConcretePointerFunctions.[hm] for the new functions  and the CallBacks 
>>> files for the old ones.
>>> 
>>> If you look at the actual hash/map table code (eg NSConcreteHashTable.m) 
>>> you will see the defines which tell GSIMAP which versions to use.
>>> eg.
>>> #define GSI_MAP_RETAIN_KEY(M, X)\
>>> (M->legacy ? M->cb.old.retain(M, X.ptr) \
>>> : pointerFunctionsAcquire(&M->cb.pf, &X.ptr, X.ptr))
>> 
>> I am not sure this helps.  The GSIMap code seems to do things like:
>> 
>> GSI_MAP_RETAIN_KEY(map, node->key)
>> node->key = key;
>> 
>> This is actually wrong in retain / release mode (-retain is not guaranteed 
>> to return self),
> 
> The guarantee is that it's specifically documented to do so in the protocol 
> .... see 
> http://developer.apple.com/library/mac/#documentation/Cocoa/Reference/Foundation/Protocols/NSObject_Protocol/Reference/NSObject.html
> So any implementation of -retain which doesn't return self is faulty, and the 
> programmer really deserves any error they have introduced by 
> overriding/replacing the default one.

My reading of that is that the requirement to return self is only a requirement 
for objects implementing their own reference counting.  The more formal 
description of -retain in the static analyser expects -retain to return an 
owning reference, but not necessarily to the same object as the receiver.  

This is used in NeXT code for immutable objects that are allocated in temporary 
zones, for example.  They return a copy on retain, so that the entire zone can 
be freed at once, giving a very primitive form of generational GC.  It's used 
in Apple's implementation of _NSConcretesStackBlock (and the one in GNUstep!), 
so that block allocated on the stack is copied to the heap if anyone retains it 
(in GC mode on OS X, you are required to make an explicit call to Block_copy(), 
with libobjc2 this is inserted by the write barrier automatically).

>> but in GC mode, these two lines need to somehow become:
>> 
>> objc_assign_strongCast(key, &(node->key));\
>> 
>> there also doesn't seem to be any macro for reading the keys.  For example, 
>> when ever you read node->key or node->value as a weak pointer, lines like:
>> 
>> GSI_MAP_EQUAL(map, node->key, key)
>> 
>> Need to be expanded to something like:
>> 
>> [objc_read_weak(&(node->key) isEqual: key]
> 
> Then we need some modification for if/when we support non-boehm GC.
> 
> The current sequence:
> 
> node->key = key;
> GSI_MAP_RETAIN_KEY(map, node->key);
> 
> could become:
> 
> GSI_MAP_ACQUIRE_KEY(map, &key_in_node, key);
> 
> and we could trivially add a macro to reference the map content.

That's more or less what I was thinking.

David

-- Sent from my Difference Engine