2-nd round RFR (S) 8008678: JSR 292: constant pool reconstitution must support pseudo strings

[serguei.spitsyn at oracle.com]

Please, review the second round fix for:
   https://bugs.openjdk.java.net/browse/JDK-8008678

Open webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2014/hotspot/8008678-JVMTI-pseudo.2/


Summary:

   This fix implements a footprint saving approach suggested by Coleen.
   To be able to reconstitute a class constant pool, an intermediate 
pseudo-string map is used.
   Now, this field is accounted optionally, only if the 'cp_patches' is 
provided in the
   ClassFileParser::parseClassFile() before ConstantPool is allocated.
   This fix is not elegant, even a little bit ugly, but it is the only 
way I see so far.

   Unfortunately, this approach did not help much to make some other 
fields (eg., 'operands') optional.
   The problem is that we have to account optional fields before 
parsing, at the CP allocation time.
   It is possible to re-allocate the ConstantPool when any InvokeDynamic 
bytecode is discovered,
   but it looks too complicated.

Testing:
   - the unit test from bug report
   - nsk.jvmti,testlist, nsk.jdi.testlist, JTREG java/lang/instrument
   - vm.mlvm.testlist, vm.quick.testlist, 
vm.parallel_class_loading.testlist (in progress)


Thanks,
Serguei


On 11/26/14 11:53 AM, serguei.spitsyn at oracle.com wrote:
> Coleen,
>
> Thank you for looking at this!
> I'll check how this can be improved.
> It is my concern too.
>
> Thanks,
> Serguei
>
> On 11/26/14 9:17 AM, Coleen Phillimore wrote:
>>
>> Serguei,
>> I had a quick look at this.  I was wondering if we could make the 
>> pseudo_string_map conditional in ConstantPool and not make all 
>> classes pay in footprint for this field?  The same thing probably 
>> could be done for operands too.  There are flags that you can set to 
>> conditionally add a pointer to base() in this function.
>>
>> Typical C++ would subclass ConstantPool to add 
>> InvokeDynamicConstantPool fields, but this is not typical C++ so the 
>> trick we use is like the one in ConstMethod.   I think it's worth 
>> doing in this case.
>>
>> Thanks,
>> Coleen
>>
>> On 11/26/14, 3:59 AM, serguei.spitsyn at oracle.com wrote:
>>> Please, review the fix for:
>>>   https://bugs.openjdk.java.net/browse/JDK-8008678
>>>
>>>
>>> Open webrev:
>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2014/hotspot/8008678-JVMTI-pseudo.1/ 
>>>
>>>
>>>
>>> Summary:
>>>    The pseudo-strings are currently not supported in reconstitution 
>>> of constant pool.
>>>
>>>    This is an explanation from John Rose about what the 
>>> pseudo-strings are:
>>>
>>>    "We still need "live" oop constants pre-linked into the constant 
>>> pool of bytecodes which
>>>     implement some method handles. We use the anonymous class 
>>> pseudo-string feature for that.
>>>     The relevant code is here:
>>> http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/tip/src/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java 
>>>
>>>     These oops are what "pseudo-strings" are.
>>>     The odd name refers to the fact that, even though they are 
>>> random oops, they appear in the constant pool
>>>     where one would expect (because of class file syntax) to find a 
>>> string."
>>>      ...
>>>     If you really wanted to reconstitute a class file for an 
>>> anonymous class, and
>>>     if that class has oop patching (pseudo-strings), you would need 
>>> either to (a) reconstitute the patches array
>>>     handed to Unsafe.defineAnonymousClass, or (b) accept whatever 
>>> odd strings were there first, as an approximation.
>>>     The "odd strings" are totally insignificant, and are typically 
>>> something like "CONSTANT_PLACEHOLDER_42"
>>>     (see java/lang/invoke/InvokerBytecodeGenerator.java)."
>>>
>>>
>>>    Reconstitution of the ConstantPool is needed for both the JVMTI 
>>> GetConstantPool() and RetransformClasses().
>>>    Finally, it goes to the ConstantPool::copy_cpool_bytes().
>>>
>>>    The problem is that a pseudo-string is a patched string that does 
>>> not have
>>>    a reference to the string symbol anymore:
>>>        unresolved_string_at(idx) == NULL
>>>
>>>    The fix is to create and fill in a map from JVM_CONSTANT_String 
>>> cp index to the JVM_CONSTANT_Utf8 cp index
>>>    to be able to restore this assotiation in the 
>>> JvmtiConstantPoolReconstituter.
>>>
>>> Testing:
>>>   Run:
>>>    - java/lang/instrument tests
>>>    - new jtreg test (see webrev) that was written by Filipp Zhinkin
>>>
>>>
>>> Thanks,
>>> Serguei
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20141217/37403a73/attachment-0001.html>