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

John Cuthbertson john.cuthbertson at oracle.com
Tue Jul 2 16:53:03 UTC 2013


Hi Vladimir,

Thank you. Ensuring I had the right hooks into the compiler code was one 
of my biggest concerns.

JohnC

On 7/1/2013 7:33 PM, Vladimir Kozlov wrote:
> John,
>
> Compiler related changes (c1_Runtime1.cpp and nmethod.cpp) look 
> reasonable.
>
> Thanks,
> Vladimir
>
> On 6/26/13 2:30 PM, 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.
>>
>> In the webrev I have folded Mikael's and Thomas' first set comments into
>> the main patch:
>>
>> http://cr.openjdk.java.net/~johnc/7145569/webrev.2/webrev.1.optimize-nmethod-scanning/ 
>>
>>
>> Thomas' second set of comments are given in:
>>
>> http://cr.openjdk.java.net/~johnc/7145569/webrev.2/webrev.2.thomas-comments-2/ 
>>
>>
>> And, as always, the whole tamale is in:
>>
>> http://cr.openjdk.java.net/~johnc/7145569/webrev.2/webrev.all/
>>
>> An example of the new RSet stats output is:
>>
>>   Concurrent RS processed 154 cards
>>    Of 21 completed buffers:
>>             21 (100.0%) by concurrent RS threads.
>>              0 (  0.0%) by mutator threads.
>>    Concurrent RS threads times (s)
>>            0.00     0.00     0.00     0.00
>>    Concurrent sampling threads times (s)
>>            0.00
>>    Total heap region rem set sizes = 231K.  Max = 1K.
>>    Static structures = 32K, free_lists = 0K.
>>      0 occupied cards represented.
>>      Max size region = 0:(O)[0xac400000,0xac446590,0xac500000], size =
>> 2K, occupied = 0K.
>>      Did 0 coarsenings.
>> /  Total heap region code-root set sizes = 10K.  Max = 0K.//
>> //    Max size region = 0:(O)[0xac400000,0xac446590,0xac500000], size =
>> 1K, num_elems = 32./
>>
>>
>> The new G1PrintRegionLivenessInfo output is:
>>
>> ### PHASE Post-Marking @ 1.673
>> ### HEAP  committed: 0xac400000-0xb5b00000  reserved:
>> 0xac400000-0xec400000  region-size: 1048576
>> ###
>> ###   type         address-range       used  prev-live
>> next-live          gc-eff     remset  code-roots
>> ###                                 (bytes)    (bytes) (bytes)
>> (bytes/ms)    (bytes)    (bytes)
>> ###   OLD  0xac400000-0xac500000     288144     284688 284688
>> 295956.8       1852        196
>> ###   FREE 0xac500000-0xac600000          0 0          0
>> 0.0       1732         76
>> ###   FREE 0xac600000-0xac700000          0 0          0
>> 0.0       1732         76
>> ###   FREE 0xac700000-0xac800000          0 0          0
>> 0.0       1732         76
>> ###   FREE 0xac800000-0xac900000          0 0          0
>> 0.0       1732         76
>> ###   FREE 0xac900000-0xaca00000          0 0          0
>> 0.0       1732         76
>> ###   FREE 0xaca00000-0xacb00000          0 0          0
>> 0.0       1732         76
>> [snip]
>> ###   FREE 0xb5700000-0xb5800000          0 0          0
>> 0.0       1732         76
>> ###   FREE 0xb5800000-0xb5900000          0 0          0
>> 0.0       1732         76
>> ###   FREE 0xb5900000-0xb5a00000          0 0          0
>> 0.0       1732         76
>> ###   FREE 0xb5a00000-0xb5b00000          0 0          0
>> 0.0       1732         76
>> ###
>> ### SUMMARY  capacity: 151.00 MB  used: 12.40 MB / 8.21 % prev-live:
>> 12.40 MB / 8.21 %  next-live: 2.67 MB / 1.77 %  remset: 0.28 MB
>> code-roots: 0.01 MB
>>
>> Testing:
>> specjvm98, client/server with -XX:+VerifyBeforeGC -XX:+VerifyAfterGC
>> -XX:+VerifyDuringGC -XX:+G1VerifyHeapRegionCodeRoots
>> -XX:+G1SummarizeRSetStats -XX:G1SummarizeRSetStatsPeriod=5
>> -XX:+G1PrintRegionLivenessInfo
>> Kitchensink (4h runs), client/server with -XX:+VerifyBeforeGC
>> -XX:+VerifyAfterGC -XX:+VerifyDuringGC -XX:+G1VerifyHeapRegionCodeRoots
>>
>> Thanks,
>>
>> JohnC
>>
>> On 6/21/2013 2:59 AM, Thomas Schatzl wrote:
>>> Hi,
>>>
>>> On Thu, 2013-06-20 at 14:00 -0700, John Cuthbertson wrote:
>>>> Hi Thomas,
>>>>
>>>> On 6/19/2013 1:33 PM, Thomas Schatzl wrote:
>>>>> Hi,
>>>>>
>>>>>     one issue I forgot to mention: currently when printing 
>>>>> remembered set
>>>>> sizes, the new nmethod remembered set is not taken into account.
>>>>>
>>>>> I am asking myselves, isn't the _strong_code_root_list not a type of
>>>>> remembered set, and shouldn't it be added to this size? What do you
>>>>> think?
>>>> I think you're right. In fact I've been calling it a sort of 
>>>> remembered set.
>>> Great.
>>>
>>>>> Additionally it would be really nice to make this information 
>>>>> available
>>>>> in some form so that further (size related) issues in that area 
>>>>> could be
>>>>> diagnosed.
>>>> In the short term we could certainly add a strong_code_root_mem_size()
>>>> routine to HeapRegion and call that from the appropriate places. It
>>>> should be straight forward - GrowableArrays keep track of their 
>>>> capacity.
>>> This seems suboptimal as this would require all places that use
>>> G1RemSet::mem_size() to add this call extra call (but possible); my
>>> intention is to include this value in regular printouts of remembered
>>> set sizes.
>>>
>>>>> Unfortunately at the moment the _strong_roots_code_list is located in
>>>>> HeapRegion, not in the remembered set, where the method to get its 
>>>>> size
>>>>> is.
>>>> Again calling a routine from the HeapRegion that "own" the RSet should
>>>> be OK.  I'll send out a new webrev when I've done this.
>>>>
>>>> Longer term moving the code roots in to the actual Rset structure (not
>>>> OtherRegionsTable though) is desirable. I'll investigate how much 
>>>> effort
>>>> it will be to this.
>>> That's exactly what I had in mind. Thanks for looking into this.
>>>
>>> Thanks,
>>>    Thomas
>>>
>>




More information about the hotspot-gc-dev mailing list