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

[Vladimir Kozlov]

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::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.
>