Hi Yumin,

Not really a review as I don't understand this part of the code.

On 17/10/2012 2:51 PM, Yumin Qi wrote:
Hi, all

May I have your codereview on

http://cr.openjdk.java.net/~minqi/8000818/
<http://cr.openjdk.java.net/%7Eminqi/8000818/>

8000818: SA constant pool need to reference to reference map after
permgen removal
Summary: After permgen removal, constant pool changed to put _ldc and
_ldc_w (fast_ldc and fast_ldcw) index to reference map, no longer
calculated via constant pool cache.

Here:

+     protected short getConstantPoolIndexFromRefMap(int rawcode, int bci) {
+         int refIndex = method.getBytecodeByteArg(bci);
+         String fmt = Bytecodes.format(rawcode);
+         switch (fmt.length()) {
+             case 2: refIndex = method.getBytecodeByteArg(bci); break;

The setting of refIndex the second time seems to be redundant.

Also, there is a mistake in 6879063: SA should use hsdis. Bytes.swap
should only check if the underlying platform is big endian since java
code follows big endian. Revert it back to its orginal form, else it
will fail ClassDump.

I think that needs its own CR. You can still combine them into a single changeset (with two bug lines).

Also this seems to indicate a gap in our testing. How did this slip through the 6879063 process? Have we closed that gap now?

Thanks,
David

Reviewed-by:
Contributed-by: [email protected]


Thanks
Yumin

Reply via email to