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

[Vladimir Ivanov]

Here's updated version:
   http://cr.openjdk.java.net/~vlivanov/8059340/webrev.01

Moved ConstantPool::_resolved_references to mirror class instance.

Fixed a couple of issues in CDS and JVMTI (class redefinition) caused by 
this change.

I had to hard code Class::resolved_references offset since it is used in 
template interpreter which is generated earlier than j.l.Class is loaded 
during VM bootstrap.

Testing: hotspot/test, vm testbase (in progress)

Best regards,
Vladimir Ivanov

On 4/21/15 9:30 PM, Vladimir Ivanov wrote:
> 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
>>