[9] RFR (S): 8059340: ConstantPool::_resolved_references is missing in heap dump

[Vladimir Ivanov]

Coleen, Chris,

I'll proceed with moving ConstantPool::_resolved_references to j.l.Class 
instance then.

Thanks for the feedback.

Best regards,
Vladimir Ivanov

On 4/21/15 3:22 AM, Christian Thalinger wrote:
>
>> On Apr 20, 2015, at 4:34 PM, Coleen Phillimore
>> <coleen.phillimore at oracle.com <mailto:coleen.phillimore at oracle.com>>
>> wrote:
>>
>>
>> Vladimir,
>>
>> I think that changing the format of the heap dump isn't a good idea
>> either.
>>
>> On 4/16/15, 2:12 PM, Vladimir Ivanov wrote:
>>> (sorry for really late response; just got enough time to return to
>>> the bug)
>>
>> I'd forgotten about it!
>>>
>>> Coleen, Staffan,
>>>
>>> Thanks a lot for the feedback!
>>>
>>> After thinking about the fix more, I don't think that using reserved
>>> oop slot in CLASS DUMP for recording _resolved_references is the best
>>> thing to do. IMO the change causes too much work for the users (heap
>>> dump analysis tools).
>>>
>>> It needs specification update and then heap dump analyzers should be
>>> updated as well.
>>>
>>> I have 2 alternative approaches (hacky and not-so-hacky :-)):
>>>
>>>  - artificial class static field in the dump ("<resolved_references>"
>>> + optional id to guarantee unique name);
>>>
>>>  - add j.l.Class::_resolved_references field;
>>>    Not sure how much overhead (mostly reads from bytecode) the move
>>> from ConstantPool to j.l.Class adds, so I propose just to duplicate
>>> it for now.
>>
>> I really like this second approach, so much so that I had a prototype
>> for moving resolved_references directly to the j.l.Class object about
>> a year ago.  I couldn't find any benefit other than consolidating oops
>> so the GC would have less work to do.  If the resolved_references are
>> moved to j.l.C instance, they can not be jobjects and the
>> ClassLoaderData::_handles area wouldn't have to contain them (but
>> there are other things that could go there so don't delete the
>> _handles field yet).
>>
>> The change I had was relatively simple.  The only annoying part was
>> that getting to the resolved references has to be in macroAssembler
>> and do:
>>
>> go through method->cpCache->constants->instanceKlass->java_mirror()
>> rather than
>> method->cpCache->constants->resolved_references->jmethod indirection
>>
>> I think it only affects the interpreter so the extra indirection
>> wouldn't affect performance, so don't duplicate it!  You don't want to
>> increase space used by j.l.C without taking it out somewhere else!
>
> I like this approach.  Can we do this?
>
>>
>>>
>>> What do you think about that?
>>
>> Is this bug worth doing this?  I don't know but I'd really like it.
>>
>> Coleen
>>
>>>
>>> Best regards,
>>> Vladimir Ivanov
>>>
>>> On 10/6/14 11:35 AM, Staffan Larsen wrote:
>>>> This looks like a good approach. However, there are a couple of more
>>>> places that need to be updated.
>>>>
>>>> The hprof binary format is described in
>>>> jdk/src/jdk.hprof.agent/share/native/libhprof/manual.html and needs
>>>> to be updated. It’s also more formally specified in hprof_b_spec.h
>>>> in the same directory.
>>>>
>>>> The hprof JVMTI agent in jdk/src/jdk.hprof.agent code would also
>>>> need to be updated to show this field. Since this is a JVMTI agent
>>>> it needs to be possible to find the resolved_refrences array via the
>>>> JVMTI heap walking API. Perhaps that already works? - I haven’t looked.
>>>>
>>>> Finally, the Serviceability Agent implements yet another hprof
>>>> binary dumper in
>>>> hotspot/agent//src/share/classes/sun/jvm/hotspot/utilities/HeapHprofBinWriter.java
>>>> which also needs to write this reference.
>>>>
>>>> Thanks,
>>>> /Staffan
>>>>
>>>> On 29 sep 2014, at 16:51, Vladimir Ivanov
>>>> <vladimir.x.ivanov at oracle.com <mailto:vladimir.x.ivanov at oracle.com>>
>>>> wrote:
>>>>
>>>>> http://cr.openjdk.java.net/~vlivanov/8059340/webrev.00/
>>>>> https://bugs.openjdk.java.net/browse/JDK-8059340
>>>>>
>>>>> VM heap dump doesn't contain ConstantPool::_resolved_references for
>>>>> classes which have resolved references.
>>>>>
>>>>> ConstantPool::_resolved_references points to an Object[] holding
>>>>> resolved constant pool entries (patches for VM anonymous classes,
>>>>> linked CallSite & MethodType for invokedynamic instructions).
>>>>>
>>>>> I've decided to use reserved slot in HPROF class header format.
>>>>> It requires an update in jhat to correctly display new info.
>>>>>
>>>>> The other approach I tried was to dump the reference as a fake
>>>>> static field [1], but storing VM internal
>>>>> ConstantPool::_resolved_references among user defined fields looks
>>>>> confusing.
>>>>>
>>>>> Testing: manual (verified that corresponding arrays are properly
>>>>> linked in Nashorn heap dump).
>>>>>
>>>>> Thanks!
>>>>>
>>>>> Best regards,
>>>>> Vladimir Ivanov
>>>>>
>>>>> [1] http://cr.openjdk.java.net/~vlivanov/8059340/static
>