RFR 8150013: ParNew: Prune nmethods scavengable list

Christian Thalinger christian.thalinger at oracle.com
Fri Mar 4 17:50:28 UTC 2016


Jon asked me to have a quick look over the code cache changes.  Looks good to me.

> On Mar 3, 2016, at 1:46 PM, Jon Masamitsu <jon.masamitsu at oracle.com> wrote:
> 
> Carsten,
> 
> Changes look good.
> 
> A couple of suggestions.
> 
> http://cr.openjdk.java.net/~cvarming/scavenge_nmethods_auto_prune/2/src/share/vm/code/codeCache.cpp.frames.html
> 
> 661     if (is_live) {
> 662       // Perform cur->oops_do(f), maybe just once per nmethod.
> 663       f->do_code_blob(cur);
> 664     }
> 665 nmethod* const next = cur->scavenge_root_link();
> 666 if (fix_relocations) {
> 667 if (!is_live || !cur->detect_scavenge_root_oops()) {
> 668 unlink_scavenge_root_nmethod(cur, prev);
> 669 } else {
> 670 prev = cur;
> 671 }
> 672 }
> 673 cur = next;
> 
> Although this does not change the actions, I find this easier to
> read.  You and Tony (and whoever else wants to) can vote
> on it.
> 
> if (is_live) { ... } else {
> if (fix_relocations) { if (cur->detect_scavenge_root_oops()) { prev = cur; } else { unlink_scavenge_root_nmethod(cur, prev);} } cur = cur->scavenge_root_link(); }
> 
> Also I'd suggest a comment before the if-test
> if you find it correct.
> 
> // Unless the relocations have been updated (fixed), the // detect_scavenge_root_oops() will not be precise so skip // the check to unlink. Note that not doing the unlinking // is safe but less optimal. if (fix_relocations) {
> 
> The comment in the header file for fix_relocations helped
> but a comment here might be helpful.
> 
> I'll sponsor.
> 
> Jon
> 
> On 2/28/2016 6:16 PM, Carsten Varming wrote:
>> Dear Hotspot developers,
>> 
>> Any chance of a review of this patch? The patch cut between 7ms and 10ms of every ParNew with one application at Twitter and I expect a 1-2ms improvement for most applications.
>> 
>> I touch the code cache and GenCollectedHeap, so I assume I need reviews from both gc and compiler reviewers. Thank you Tony Printezis for the review (posted on the hotspot-gc-dev list).
>> 
>> I also need a sponsor.
>> 
>> Carsten
>> 
>> On Fri, Feb 19, 2016 at 10:52 AM, Carsten Varming <varming at gmail.com <mailto:varming at gmail.com>> wrote:
>> 
>>    Dear Hotspot developers,
>> 
>>    I would like to contribute a patch for JDK-8150013
>>    <https://bugs.openjdk.java.net/browse/JDK-8150013>. The current
>>    webrev can be found here:
>>    http://cr.openjdk.java.net/~cvarming/scavenge_nmethods_auto_prune/2/
>>    <http://cr.openjdk.java.net/%7Ecvarming/scavenge_nmethods_auto_prune/2/>.
>> 
>>    Suggestions for improvements are very welcome.
>> 
>>    Carsten
>> 
>> 
> 



More information about the hotspot-dev mailing list