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

Ivan Maidanski ivmai at mail.ru
Wed Nov 14 02:54:48 PST 2012


Hi Zach,

Your commits look good (I haven't run make check yet). Please go ahead.

Some minor thing to change:

1. type: git config user.name "Zach Saw"
2. do not forget to CC to the mailing list
3. do forget to put link to diff to the letter (like for these 2 your commits - https://github.com/zachsaw/bdwgc/compare/3d097c8c92...d8e15a91c2)
4. Rename dl_hashtbl -> dl_hashtbl_s (no separate commit is needed for such tiny changes)
5. Replace /* head */ 0 -> /* head */ NULL (I prefer to distinguish pointers and zero number)

Thank you.

Regards,
Ivan

 14 11 2012 16:46:33 Zach Saw <zach.saw at gmail.com>:
>	
>
>
	
	
>
		
		
			
>Hi Ivan,
>
>
>I've refactored the hash table vars as you asked.
>
>
>Please do a code review:
>https://github.com/zachsaw/bdwgc/tree/add-long-weakref
>
>
>
>p.s. long weakref is a term by Microsoft -- http://msdn.microsoft.com/en-us/library/ms404247.aspx
>
>
>
>In fact, a google search on long weakref will show you what it is. I think calling it a long weakref is a good name since people who are not familiar with it can google it.
>
>
>Regards,
>Zach
>
>On Wednesday, November 14, 2012, Ivan Maidanski  wrote:
>
>>Hi Zach,
>>
>>No. Just push you commit to that branch in your working tree on github and let me know:
>>
>>git clone https://github.com/<your_name>/bdwgc.git
>>
cd bdwgc
>>git checkout -b add-long-refs
>>// edit code and make check
>>git commit
>>git push
>>
>>Regards,
>>Ivan
>>
>>12 12 2012 23:32:23 от Zach Saw <zach.saw at gmail.com>:
>>
	
>>>
>>>
	
	
>>>
		
		
			
>>>Hi Ivan,
>>>
>>>How do I create a branch for add-long-weakref? In order for code review, it needs to be pushed to remote right?
>>>
>>>Zach
>>>
>>>
>>>
>>>On Tue, Nov 13, 2012 at 8:52 PM, Ivan Maidanski <ivmai at mail.ru> wrote:
>>>
>>>>Hi Zach,
>>>>
>>>>The general algorithm of the patch is ok. But I can't accept this patch without significant refactoring because:
>>>>* it contains both code refactoring and feature addition in a single patch
>>>>* the patch is too big for eye-inspection
>>>>

* it contains big #defines (a 5-line define is ok but bigger is questionable) - complicates stepping thru the code in debugger as well as static code analysis
>>>>* code adding long-refs functionality should be put in some NOT_NEEDED ifdef (similar to GC_MOVE_DISAPPEARING_LINK_NOT_NEEDED or JAVA_FINALIZATION_NOT_NEEDED).
>>>>
>>>>Could I ask you to perform some refactoring of existing code in finalize.c before adding new feature?
>>>>If yes, please follow these steps:
>>>>1. clone bdwgc repository at github (exactly this service is not strictly necessary but should simplify code reviewing)
>>>>

2. checkout some new development branch (e.g., "add-long-ref")
>>>>3. Replace put dl_head, log_dl_table_size and dl_entries into single structure, define GC_dl_hashtbl global var instead of 3 ones (code refactoring, step 1)
>>>>

4. Wait for code review results
>>>>5. Move most code of GC_general_register_disappearing_link (except for arguments checks and locks) to new GC_register_disappearing_link_inner (it could be GC_INLINE) which should accept pointer to GC_dl_hashtbl (in addition to link and obj);
>>>>

same for GC_unregister_disappearing_link and GC_move_disappearing_link (code refactoring, step 2)
>>>>6. Wait for code review results and propose idea (without writing the code immediately) how to refactor GC_finalize to simplify adding long-ref functionality
>>>>

....
>>>>
>>>>Also, about "long ref" term: is there some better naming (as I understand this is close to Java Phantom refs but phantom might be not descriptive as well). Could you point me to the place from which you took "long ref" term? Any ideas for better naming?
>>>>
>>>>And, please ask if something is unclear before writing significant amount of the code (there should be no rush).
>>>>
>>>>Thank you. Sorry about loading with "extra" refactoring tasks before you get the requested functionality added to GC.
>>>>
>>>>PS. Having added new functionality, then, next GC release, we could think about more efficient finalization/weak_refs implementation (preserving public API).
>>>>
>>>>Regards,
>>>>Ivan
>>>>
>>>>Mon, 12 Nov 2012 17:16:14 +1100 Zach Saw <zach.saw at gmail.com>:
>>>>
	
>>>>>
>>>>>
	
	
>>>>>
		
		
			
>>>>>
>>>>>
>>>>> Hi Hans,
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>-         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?
>>>>>It turns out you're right. It is indeed as simple as that!
>>>>>
>>>>>
>>>>>
>>>>>Here's the simplified patch. Can't believe it's actually so simple to add long weakrefs support directly into the GC.
>>>>> 
>>>>>Zach 
			
		
		
	

	
>>>>
>>>
			
		
		
	

	
>>
			
		
		
	

	
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://napali.hpl.hp.com/pipermail/gc/attachments/20121114/4cc98863/attachment.htm


More information about the Gc mailing list