Unfortunately, it broke class redefinition (see JDK-8081004 [1]) and I don't see a reasonable way to fix that.

The problem is that multiple class versions can be alive at the same moment w.r.t. class redefinition. Every class version should have its dedicated set of resolved_reference slots for invokedynamic/invokehandle instructions. They still have dedicated constant pools (and hence constant pool caches), but resolved_references array is shared now.

It's not viable to simply append slots to the array during class redefinition since it causes memory leak (array size grows indefinitely). Compactification strategies are also complex, since slot indices should be either (1) stable (they are recorded in CPCEs) and slots are reference counted (in order to free and reuse them on next class redefinition); or (2) all class versions are patched with new indices on every class redefinition.

IMO both approaches complicate things too much compared to keeping dedicated resolved_references per ConstantPool instance.

Serguei, Staffan, what do you think? Do you see any viable solutions to the problem?

Otherwise, I'm inclined to backout the change.

Best regards,
Vladimir Ivanov

[1] https://bugs.openjdk.java.net/browse/JDK-8081004

On 5/18/15 8:32 PM, Vladimir Ivanov wrote:
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.phillim...@oracle.com <mailto:coleen.phillim...@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.iva...@oracle.com <mailto:vladimir.x.iva...@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

Reply via email to