JDK-8155673 : Remove constant pool merging for class redefinition
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: jvmti
  • Affected Version: 9
  • Priority: P3
  • Status: Closed
  • Resolution: Won't Fix
  • Submitted: 2016-04-28
  • Updated: 2020-01-14
  • Resolved: 2020-01-14
Related Reports
Blocks :  
JDK-8207198 - Parsing annotations depends on internal VM constant pool
Relates :  
JDK-5088035 - hotswap fires assert(0 <= i && i < length(),"index out of bounds")
Relates :  
JDK-8138588 - VerifyMergedCPBytecodes option cleanup needed
Relates :  
JDK-8140685 - Fix backtrace building to not rely on constant pool merging
Description
Find way to support class redefinition without constant pool merging which has introduced some bugs, is not thread safe and makes it difficult to make changes in the constant pool without adding support for redefinition (like jsr292).
Comments
I'm closing this as WNF. People have been merging constant pool entries for new features, so it hasn't been an undue burden. The new features for constant pools in Java seem to rely on the notion of constant pool merging - ie constant pool entry at index n is consistent across bytecodes and safepoints.
14-01-2020
I examined the issue with removing constant pool merging with jdk.internal.reflect.ConstantPool. This java object used to have a reference to the ConstantPool* metadata itself, but since permgen elimination, has a reference to the java.lang.Class object (ie. mirror). The JVM functions retrieve the constant pool from the mirror object, ie: InstanceKlass::cast(java_lang_Class::as_Klass(mirror))->_constants() The code uses jdk.internal.reflect.ConstantPool to parse annotation entries, which are also loaded from the InstanceKlass object. Currently if a class is redefined, the constant pool fetched is the merged constant pool, where new constant pool entries are appended to the old constant pool. The code that loads the annotations before and after redefinition will always point to some entry in the merged constant pool. ie from Class.java: private AnnotationData createAnnotationData(int classRedefinedCount) { Map<Class<? extends Annotation>, Annotation> declaredAnnotations = AnnotationParser.parseAnnotations(getRawAnnotations(), getConstantPool(), this); ... If a redefinition happens between getRawAnnotations() and getConstantPool() calls, the byte array in the annotations (the bytes point to constant pool indices) will point to some valid entry in the constant pool returned later even after redefinition when the constant pools are merged. If the constant pools are not merged, the getConstantPool() call will return a constant pool that may not have the original annotation entries. You could fix this by making the jdk.internal.reflect.ConstantPool point to the "old" constant pool, and making the calls to get the annotation array and getting the constant pool appear as-if atomic. One way to do this is to query the classRedefinedCount between these calls. There are a few places in the jdk code where this would have to be done. This is sort of manageable. The real solution is to rewrite annotation processing to NOT have access to JVM internal data structures! If jdk.internal.reflect.ConstantPool pointed to the ConstantPool itself, the vm would have to have a WeakHandle hashtable to keep track of jdk.internal.reflect.ConstantPool object so we mark them as on_stack() so we don't deallocate the old constant pool if some java object is referring to it. Patch with changes excluding constant pool handling is here: http://cr.openjdk.java.net/~coleenp/8155673.01/webrev/index.html I'm moving this to tbd_major, but it's blocked by this RFE I just filed. https://bugs.openjdk.java.net/browse/JDK-8207198
12-07-2018
I have a patch that I've been testing for a long time. It needs a bit more work for java.lang.reflect.ConstantPool instances though. Can you mark this for 13? Or if someone would like to complete my patch, I'd be willing to give it away.
10-07-2018
Coleen: Are you going to work on this for JDK12?
10-07-2018
Yes, there are technical hurdles, which is why this is for jdk10. 1. old methods point to the original constant pools, so that's ok. I wanted the original constant pools to point to the scratch_class with the old contents in it, but this didn't work where it is in the code currently. So there are some pointers that need to be followed and verified that you can't get to the new class with an old method. The old method currently keeps the scratch_class alive from deallocation, due to some subtlety that I want to make not so subtle. 2. so old methods can be invoked if they're too far in the invocation (already in the register to invoke). They'll just point to the old constant pools. I have a prototype of this that passes all the tests but of course, we need to inventory the tests and make sure we're covering the tricky cases above. There are a lot of good stress tests though and ones that combined setting breakpoints and watchpoints with redefinition. Again though we'd have to verify test coverage to make sure we're handling the corner cases.
04-05-2016
Technical hurdles: 1) What do you do about a method that has been redefined, but is still being executed by a thread? If the method has an internal loop so that it never returns to its caller, then the old method will never go away. An example would be a work dispatch method that sits in a loop reading operations from a socket and then dispatches those operations to other threads. 2) Similarly, what do you do about a method call that is in the process of being invoked, pauses for the safepoint which redefines the method, but the invoke is already past the point of finding the newly redefined method so it makes one last call to the old method? There are probably more rattling around somewhere in my memory and I'll update this note as a I remember them... In short: removing constant pool merging will not be an easy or risk free task. There be dragons here...
28-04-2016