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