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

Thomas Schatzl thomas.schatzl at oracle.com
Thu Jun 27 09:11:32 UTC 2013


Hi,

On Wed, 2013-06-26 at 14:30 -0700, John Cuthbertson wrote:
> Hi Everyone,
> 
> A new version of the changes can be found at:
> http://cr.openjdk.java.net/~johnc/7145569/webrev.2/
> 
> Changes in this version include:
> 
> * The verification code blob closure in g1CollectedHeap.cpp now
> inherits directly from CodeBlobClosure rather than
> CodeBlobToOopClosure to address Mikael's final comment.
> * The list of nmethods has been moved from the HeapRegion class to the
> HeapRegionRemSet class. The routines in HeapRegion are now, as Thomas
> suggested, wrappers and invoke the associated method from the RSet
> owned by the heap region.
> * Added a routine to return the number of bytes currently occupied by
> the list of nmethods and used that in the G1SummarizeRSetStats output
> and the G1PrintRegionLivenessInfo output. I've been back and forth
> about whether to include the size of the memory occupied by the code
> roots into the value returned by HeapRegionRemSet::mem_size(). In this
> implementation I've included it but also included separate output for
> it in RSet stats and region liveness output.

> And, as always, the whole tamale is in:
> 
> http://cr.openjdk.java.net/~johnc/7145569/webrev.2/webrev.all/

heapRegionRemSet.cpp:

1027 +  // FIXME: assert that region got an evacuation failure if
non-empty

Maybe:

assert(to_be_retained.is_empty() || hr()->evacuation_failed(), "Retained
nmethod list must be empty or the evacuation of this region failed");

(Did not test :)

g1CollectedHeap.cpp:

1182       // We'll assert that the strong code root list is empty
1183       assert(hrrs->strong_code_roots_list_length() == 0, "sanity");

This is more a comment: I think you could add hrrs->occupied() == 0 as
condition here.

Looks good. Thanks for considering my comments.

Thomas





More information about the hotspot-gc-dev mailing list