[Gc] Re[4]: Patch for native support of long weakrefs

Ivan Maidanski ivmai at mail.ru
Thu Nov 15 01:59:47 PST 2012


Hi Zach,

15 11 2012 20:32:05 Zach Saw <zach.saw at gmail.com>:
>Hi Ivan,
>>
>
> 1. You latest patch requires minor fixing (no need to repost this patch, just make another patch fixing this problem):
>
> DCL_LOCK_STATE declares any local variables needed by LOCK and UNLOCK, so DCL_LOCK_STATE should be placed there LOCK/UNLOCK are (as I understand we could deprecate/remove DCL_LOCK_STATE but not now).
>
>
>
> More refactoring:
>
> 2. GC_register_disappearing_link_inner: we can't move out locking from this function, so please move argument checking back to it (just to avoid code duplication)
>
>
>
> 3. GC_move_disappearing_link_inner: make lock/unlock out of this function (so that GC_move_disappearing_link will do lock/call GC_move_disappearing_link_inner/unlock/return res.
To kill 2 birds with one stone - make the code simpler and be ready do add long refs:

GC_move_disappearing_link code at present:
LOCK
...
if (..) {UNLOCK; return}
...
if (..) {UNLOCK; return}
...
if (..) {UNLOCK; return}
...
UNLOCK

I suggest:
GC_move_disappearing_link_inner: no LOCK/UNLOCK

GC_move_disappearing_link:
int res;
DCL_LOCK_STATE;
// check args
LOCK
res = GC_move_disappearing_link_inner
UNLOCK
return res;

>Why the inconsistency? Why don't we move all of the code into the inner functions consistently? And when we introduce the new long weakref, it won't have any code duplications.
>> 4. GC_unregister_disappearing_link_inner: I think it's better to move lock/unlock from it as well but, if so, it should return curr_dl (instead of 1) or NULL, while GC_unregister_disappearing_link should do arg checking, lock/unlock and GC_free/dl_set_next (for the latter operation, I suggest do define a macro like GC_FREE_DL_ENTRY - as I recall one if your submitted patches (with big defines) contained such macro).
>If you agree with consistency argument above, this would be the same.
As I said I doubt about GC_unregister_disappearing_link_inner - ok let's put all code back to it.

Regards,
Ivan
>Zach

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://napali.hpl.hp.com/pipermail/gc/attachments/20121115/1394ad48/attachment-0001.htm


More information about the Gc mailing list