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

Coleen Phillimore coleen.phillimore at oracle.com
Wed May 22 04:21:12 PDT 2013


On 5/22/2013 2:59 AM, Staffan Larsen wrote:
> Looks good.

Thank you, Staffan.
>
> Can you file a bug on SA that we should look at fixing up the hprof entries correctly?

Yes, I'll do that.

Thanks,
Coleen

>
> Thanks,
> /Staffan
>
> On 21 maj 2013, at 21:47, Coleen Phillimore <coleen.phillimore at oracle.com> 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.
>>
>> http://cr.openjdk.java.net/~coleenp/8003421_2/
>>
>> 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



More information about the hotspot-runtime-dev mailing list