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

[Vladimir Kozlov]

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::make_from_constant to succeed for user-provided CallSite objects.

make_from_constant() is called with require_constant == true for invokedynamic 
CallSites and it calls can_be_constant() 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/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.
>