[Gc] Re: Patch for native support of long weakrefs
hans.boehm at hp.com
Sun Nov 11 14:37:36 PST 2012
Some more comments:
- The comment for GC_general_register_disappearing_link_ex (or preferably a separate replacement function as Ivan suggested) needs to be much more careful in describing the functionality. I’m not sure I understand yet. You want to remove links if objects are no longer reachable from anything, including from finalizable objects? Isn’t that easier to determine after all finalizable objects have been traced?
- The comment describing the _hb_finalized array needs to be fixed.
- This patch seems to unconditionally add significant space overhead (1/8 or 1/16th of the heap) to support an esoteric feature that is unlikely to be used by non-Mono clients, and is probably rarely used by Mono. I think that’s not acceptable. I share Ivan’s doubts about whether it’s actually needed. If so, we would at least need a way to allocate these bits only conditionally.
From: gc-bounces at linux.hpl.hp.com [mailto:gc-bounces at linux.hpl.hp.com] On Behalf Of Ivan Maidanski
Sent: Friday, November 09, 2012 11:45 AM
To: Zach Saw
Cc: Boehm GC
Subject: [Gc] Re: Patch for native support of long weakrefs
Fri, 9 Nov 2012 23:05:10 +1100 Zach Saw <zach.saw at gmail.com<mailto:zach.saw at gmail.com>>:
What do you mean by "truly inaccessible"? If an object is accessible via hidden pointer, is it truly inaccessible?
If it is accessible via hidden pointer, it is not accessible. Truly inaccessible in the sense of a normal precise GC where objects no longer have any possibilities of being traceable from a root set.
This functionality could be emulated via finalizers but it is not performance efficient. As I understand, it worth adding to GC but Hans might have a different opinion.
Yes, indeed this functionality was emulated via finalizers in Mono ***but*** have you looked at how convoluted / functionalities replicated from the short weakref within BoehmGC is required to achieve it? Performance one issue, code convolution / bloat another.
Comments on patch:
1. I think it's better to propose a separate function instead of a parameter because:
It is already a separate function with the '_ex' suffix, isn't it?
I've meant two 2-argument functions - one is GC_general_register_disappearing_link and the other is for long-term refs.
2. it's better to use a separate table for storing "long-term" refs (otherwise GC will scan the joint table twice, or even, 3 times)
I agree. I initially did that (as I implemented two separate lists in my own GC without the need to use a hashtable -- it is a lot faster to create/delete links) but I had to refactor a lot of code to reduce code repetition and create a new function for each of the register/unregister/move disappearing_link function. Can you help me with this one please? I abandoned my initial effort due to the fact that I do not know what your coding standards are for refactoring the code particularly in cases where I have to move code into function.
I agree some code refactoring is needed (for register/unregister/move).
3. Is GC_clear_finalized_bit (and friends) actually needed?
Yes. It is the finalized_bit in the block header that keeps track of whether an object has been finalized. Combined with the object's mark bit, we then determine if we need to clear the long term link.
4. No need to define GC_general_register_disappearing_link redirected to GC_general_register_disappearing_link_inner
What's the alternative? Have GC_general_register_disappearing_link call GC_general_register_disappearing_link_ex?
Yes. But if you agree with comment item 1 then this goes away.
5. There is some code duplication in the patch (Make disappearing links disappear), is there a way to eliminate it?
They are not actually duplication -- the only duplication is that they iterate the hash table, but each time for a different reason, thus performing different tasks.
The first time (original one) it scans short-term links.
Second time, long-term links.
Third time, getting rid of long-term links whose object has no chance of getting resurrected.
Again, we need refactoring of existing code (e.g., propose some macros to iterate the table).
6. Please clean up your patch removing such non-functional changes from:
- /* Field to be cleared. */
+ /* Field to be cleared. */
- (*(curr_fo -> fo_fn))((ptr_t)(curr_fo -> fo_hidden_base),
+ real_ptr = (ptr_t)(curr_fo -> fo_hidden_base);
+ (*(curr_fo -> fo_fn))(real_ptr,
On a separate note, I also feel like using a hashtable for disappearing links is overkill in memory usage and is slow.
In my implementation, I use a linear vector and return the index to the vector with its MSB 1 to indicate long weakref (a HANDLE) when you allocate a weakref.
HANDLE GCHandle::InternalAlloc(void* target, bool trackResurrection)
void GCHandle::InternalFree(HANDLE handle)
void* GCHandle::InternalGet(HANDLE handle)
void* GCHandle::InternalUpdate(HANDLE handle, void* value)
There's also no need to update an external memory from (i.e. link) within the GC which may end up causing access violation.
All weakrefs are obtained via InternalGet which simply returns NULL when it's been cleared.
In general, comparing the performance of creating and removing weakrefs, BoehmGC is painfully slow.
The rest are equal as the tasks required in finalization step remains the same.
If it is possible, I propose we drop the old hashtable based disappearing link algo and adopt the one I described above.
Probably. Let's wait for comments from Hans.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the Gc