RFR 8150013: ParNew: Prune nmethods scavengable list

Carsten Varming varming at gmail.com
Fri Mar 4 18:23:57 UTC 2016


Dear Christian,

Thank you for your review.

Carsten

On Fri, Mar 4, 2016 at 12:50 PM, Christian Thalinger <
christian.thalinger at oracle.com> wrote:

> 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