RFR 8003421: NPG: Move oops out of InstanceKlass into mirror

[Stefan Karlsson]

On 5/21/13 9:47 PM, Coleen Phillimore wrote:
>
> Thanks Stefan, Staffan, Dean, and David for comments.   I have a new 
> webrev and have tested this new change that doesn't break hprof format 
> in the SA and fixed things from the code reviews.  I didn't move 
> component mirror.  I have to think about this more. Please re-review.

I didn't expect you to move the component mirror as a part of this 
change. It was just a hint that there's one more oop that we might want 
to move.

>
> http://cr.openjdk.java.net/~coleenp/8003421_2/

Looks good.

thanks,
StefanK

>
> Thanks,
> Coleen
>
>
> On 05/21/2013 11:01 AM, Coleen Phillimore wrote:
>>
>> On 05/21/2013 09:18 AM, Staffan Larsen wrote:
>>>>>> Net footprint change is zero except that these fields are in Java 
>>>>>> heap
>>>>>> rather than metaspace.  This helps a little with InstanceKlass size
>>>>>> which is in fixed size space with UseCompressedKlassPointers. 
>>>>>> Included
>>>>>> serviceability because there were SA changes to code that I don't 
>>>>>> know
>>>>>> is used.
>>>>> Unsure about the SA changes. Basically you just removed access to 
>>>>> the pd and signers, rather than changing it to allow access via 
>>>>> the new path. That said I don't know SA so don't know whether it 
>>>>> makes sense for SA to access things that are logically part of 
>>>>> java.lang.Class; or whether it can access them more directly 
>>>>> anyway because they are logically part of java.lang.Class.
>>>>>
>>>> I did remove these from instanceKlass.  It doesn't appear in the SA 
>>>> that it digs into the java mirror so there was nowhere to put these 
>>>> fields.   The code that I took out was in places that may not be 
>>>> used and may be bit rotted.   I added serviceability team to the 
>>>> review request so someone could comment.  I already asked Staffan 
>>>> about this.
>>> Reading this in more detail, I'm a little worried about the change 
>>> in hprof output. The change in HeapHprofBinWriter actually breaks 
>>> the hprof binary format, leading to unparseable files. Granted, this 
>>> is hprof output when invoked either in SA or with "jmap -F" so it 
>>> won't be used by very many people and I can't find any tests that do 
>>> this.
>>>
>>> I think we need to find a way to add this information back.
>>
>> Hi,  I didn't want to break the hprof format so I added it back to 
>> return null for protection domain and signers.  There doesn't seem to 
>> be a way in SA to read from the mirror but I posted suggested code 
>> once this sort of thing is implemented.   See:
>>
>> http://cr.openjdk.java.net/~coleenp/8003421_2/agent/src/share/classes/sun/jvm/hotspot/utilities/HeapGXLWriter.java.udiff.html 
>>
>> http://cr.openjdk.java.net/~coleenp/8003421_2/agent/src/share/classes/sun/jvm/hotspot/utilities/HeapHprofBinWriter.java.udiff.html 
>>
>>
>> I can file a bug against the SA to get the correct information or 
>> decide whether this is something you need to support in this manner.
>>
>> thanks,
>> Coleen
>>
>>>
>>> /Staffan
>>
>