Request for reviews (XS): 7048030: is_scavengable changes causing compiler to embed more constants (original) (raw)

Vladimir Kozlov vladimir.kozlov at oracle.com
Tue May 24 20:26:46 PDT 2011

Thank you, John

Vladimir

John Rose wrote:

Yes; thanks. You can count me as a reviewer. -- John

On May 24, 2011, at 6:36 PM, Vladimir Kozlov wrote:

Tom Rodriguez wrote: On May 24, 2011, at 6:00 PM, John Rose wrote:

If this affects the processing of 292 data, there may be significant performance regressions. The key uses of embedded non-perms are invokedynamic CallSites, which requires TypeOopPtr::makefromconstant to succeed for user-provided CallSite objects. makefromconstant() is called with requireconstant == true for invokedynamic CallSites and it calls canbeconstant() which will return true for ScavengeRootsInCode >= 1. So it should continue to work for 292. Vladimir

But, I think this will continue to work if ScavengeRootsInCode is not zero. Do you agree? It should restore the logic to how it used to work prior to 7041789. http://hg.openjdk.java.net/jdk7/hotspot-gc/hotspot/raw-diff/2aa9ddbb9e60/src/share/vm/gcinterface/collectedHeap.hpp @@ -284,11 +291,7 @@ class CollectedHeap : public CHeapObj { // An object is scavengable if its location may move during a scavenge. // (A scavenge is a GC which is not a full GC.) - // Currently, this just means it is not perm (and not null). - // This could change if we rethink what's in perm-gen. - bool isscavengable(const void *p) const { - return !isinpermanentornull(p); - } + virtual bool isscavengable(const void *p) = 0; 292 ignores shouldbeconstant anyway. tom -- John

P.S. FTR, some of history behind isscavengable and isperm is in this discussion, relating to 6863023: http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2009-September/001261.html On May 24, 2011, at 5:19 PM, Vladimir Kozlov wrote:

http://cr.openjdk.java.net/~kvn/7048030/webrev

Fixed 7048030: isscavengable changes causing compiler to embed more constants This is the cause of 7047300 problem to appear. The fix for 7041789 corrected the meaning of isscavengable to really mean what it says. Unfortunately there were some places that were really using it as a proxy for !isperm. In particular in ciObject::canbeconstant and shouldbeconstant we will now embed constants that we didn't used as long as they are in tenured. The code should be changed from !isscavengable to isperm. Verified with failing test from 7047300.

More information about the hotspot-compiler-dev mailing list