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

[Tom Rodriguez]

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::make_from_constant to succeed for user-provided CallSite objects.
> 
> 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/gc_interface/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 is_scavengable(const void *p) const {
-    return !is_in_permanent_or_null(p);
-  }
+  virtual bool is_scavengable(const void *p) = 0;

292 ignores should_be_constant anyway.

tom

> 
> -- John
> 
> P.S.  FTR, some of history behind is_scavengable and is_perm 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: is_scavengable changes causing compiler to embed more constants
>> 
>> This is the cause of 7047300 problem to appear.  The fix for 7041789 corrected the meaning of is_scavengable to really mean what it says.  Unfortunately there were some places that were really using it as a proxy for !is_perm.  In particular in ciObject::can_be_constant and should_be_constant we will now embed constants that we didn't used as long as they are in tenured.  The code should be changed from !is_scavengable to is_perm.
>> 
>> Verified with failing test from 7047300.
>