RFR(M/L): 7145569: G1: optimize nmethods scanning

John Cuthbertson john.cuthbertson at oracle.com
Wed Jun 19 20:07:08 UTC 2013


Hi Thomas.

Inline....
>> I would rather not have "registration" dependent upon not being on the
>> scavengable nmethod list. I can see in the future that registration may
>> actually perform adding to the scavengable list but I would like to keep
>> them separate for now.
>>
> Okay.

Thanks!

>>> - In HeapRegion::hr_clear(), it may be interesting to not only clear the
>>> code root list, but deallocate it. Depending on the size of this list
>>> (do you have numbers?) this may lead to excessive memory use of free
>>> regions' HeapRegions. I.e. there have been too many issues recently
>>> about G1 taking too much space by default :) In the same direction,
>>> maybe the default size of 10 could be lowered. Eg. with 60k+ region
>>> heaps, this will add 4M+ to the heap at startup.
>>> Then again, this could be done in another cr when fixing the general
>>> issue.
>> I'm OK with this. The down side is that
>> GrowableArray::clear_and_deallocate() is private so I have to delete the
>> pointer and allocate anew. Is this OK?
> This is fine with me.

Cool.

>
>>> - in remove_strong_code_root() the code tries to remove multiple entries
>>> of a given nmethod. How is it possible to get multiple duplicate
>>> entries, assuming that push_strong_code_root() already does not push an
>>> nmethod if it is already in the list?
>> Good point. I don't think it should. Previously I was only checking the
>> top most element in the list for duplication. Is it better to defend
>> against it or just guarantee it?
> I am not sure what you mean here in terms of code... :) Without looking
> again when is called what, I would simply try to avoid adding duplicates
> into the lists, and maybe in debug mode check that during removal.

I mean the current code removes duplicates and so is defensive. Adding a 
guarantee that there are other entries after removing the first is 
enforcement.  The current add routine checks the entire list before 
adding a new entry so duplicate entries should be avoided. I'll change 
the routine and go the enforcement route.

A new webrev with an unchanged version of 
HeapRegion::remove_strong_code_root() can be found at: 
http://cr.openjdk.java.net/~johnc/7145569/webrev.1/

The changes associated with your comments are in: 
http://cr.openjdk.java.net/~johnc/7145569/webrev.1/webrev.2.thomas-comments/
The changes associated with Mikael's comments are in: 
http://cr.openjdk.java.net/~johnc/7145569/webrev.1/webrev.3.mikaelg-comments/

JohnC





More information about the hotspot-gc-dev mailing list