RFR (XXL) [7u60]: nmethod backports (12 backports) (original) (raw)

Thomas Schatzl thomas.schatzl at oracle.com
Thu Jan 23 16:43:45 UTC 2014


Hi,

thanks for your review :)

On Tue, 2014-01-21 at 20:04 -0800, Vladimir Kozlov wrote:

Thomas,

Based on your tests list you sent I thinks you get good test coverage. Thanks! I compared compared these changes with jdk8 changes. c1Runtime1.cpp: missed {} parenthesis.

Fixed.

nmethod.cpp: nmethod::oopsdo() changes. The only place where dostrongrootsonly was passed as 'true' was in donewlymarkednmethod(). But you changed it to 'false': void CodeBlobToOopClosure::donewlymarkednmethod(nmethod* nm) { - nm->oopsdo(cl, /dostrongrootsonly=/ true); + nm->oopsdo(cl, /dostrongrootsonly=/ false, /allowzombie=/ false); } Does it make sense to keep the argument?

This is a bug actually, the second parameter should read true. It's a performance issue though only, so it went undetected so far. It prevents some unloading. Thanks for finding this!

I fixed both issues in http://cr.openjdk.java.net/~tschatzl/nmethod-backport/2/ , or http://cr.openjdk.java.net/~tschatzl/nmethod-backport/2/webrev.05-7145569 (direct link to webrev). No other changes.

Ad-hoc testing (including bigapps, e.g. kitchensink, weblogic, runthese 30mins) is currently running, with 90% complete and no issues so far (g1 product/debug, and default collector with product/debug builds). I do not expect any surprises here.

The previous build also passed 24h kitchensink on all tested platforms (only did a few, not all) with both collectors.

Thanks a lot, Thomas



More information about the hotspot-gc-dev mailing list