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

[John Cuthbertson]

Hi Thomas,

I incorporated you're last two comments and retested. I used your 
suggested assert in heapRegionRemSet.cpp and added the occupied count == 
0 for humongous regions as a separate assert. I used jprt and 
kitchensink to retest.

Thanks for the review.

JohnC

On 6/27/2013 2:11 AM, Thomas Schatzl wrote:
> 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
>
>