RFR: 8074345: Enable RewriteBytecodes when VM runs with CDS

Coleen Phillimore coleen.phillimore at oracle.com
Tue Mar 10 21:54:42 UTC 2015


Yumin,

The new bytecode approach came out pretty cleanly, or as cleanly as this 
could be.

The file templateTable_x86_32 and 64 have just been merged, so you'll 
have to make your change in the new version.

I also have some comments:

http://cr.openjdk.java.net/~minqi/8074345/src/share/vm/interpreter/rewriter.cpp.udiff.html

Typo "rewirting"

In these files, can you break up the long lines into three lines?

-  if (!is_static) { patch_bytecode(Bytecodes::_fast_fgetfield, Rbc, Rscratch); }
+  if (!is_static && rc == MAY_REWRITE) { patch_bytecode(Bytecodes::_fast_fgetfield, Rbc, Rscratch); }


http://cr.openjdk.java.net/~minqi/8074345/src/share/vm/interpreter/bytecodes.hpp.udiff.html

How many bytecodes do we have now?  We're limited to 255 (or 256) and 
there are other new bytecodes being added.

What was the performance benefit to this?   I think if we wanted to be 
conservative, we'd turn off RewriteFrequentPairs and only do 
nofast_getfield and nofast_putfield.  I think they were the only 
bytecodes that actually affected performance.

In this file above, can you remove the last block of comments about 
fast_linearswitch and fast_ldc?  I think this confuses rewriting in the 
interpreter and rewriting in the rewriter, or rather makes the confusion 
worse.  I don't think this comment is helpful.

I'd prefer to see the first comment smaller also, like:

+    // These bytecodes are rewritten at CDS dump time, so that we can prevent them from being
+    // rewritten at run time. This way, the ConstMethods can be placed in the CDS ReadOnly
+    // section, and RewriteByteCodes/RewriteFrequentPairs can rewrite non-CDS bytecodes
+    // at run time.
+    _nofast_getfield      ,
+    _nofast_putfield      ,
+    _nofast_aload_0       ,
+    _nofast_iload         ,
+    _nofast_invokevirtual ,

It's sort of obvious which bytecode they rewrite.  I don't know how much 
performance fast_invokevfinal is worth.  I thought I deleted it.  Can we 
not rewrite this so we don't waste another bytecode on it?  Maybe add a 
RewriteVFinal option and consider removing it for the future?  x86 
doesn't use it and I can't see how this would save any significant 
performance to be worth having!

http://cr.openjdk.java.net/~minqi/8074345/src/share/vm/interpreter/templateTable.hpp.udiff.html

+  enum RewriteControl { MAY_REWRITE, MAY_NOT_REWRITE };  // control for fast code under CDS


I don't know what our coding standard is but in the 
templateTable_<cpu>.cpp files these strings look like macros. I'd rather 
see them as MayRewrite or MayNotRewrite.

http://cr.openjdk.java.net/~minqi/8074345/src/cpu/sparc/vm/templateTable_sparc.cpp.udiff.html

I think there's a java_code() function that returns the original 
bytecode that you could use instead of the case statement in 
resolve_cache_and_index().   The indentation is odd in the webrev. This 
probably applies to the other cpu directories.

One last question below:

On 3/5/15, 4:21 PM, Yumin Qi wrote:
> Please review:
>
> bug: https://bugs.openjdk.java.net/browse/JDK-8074345
> webrev: http://cr.openjdk.java.net/~minqi/8074345/
>
> Summary: Currently CDS when is disabled, RewriteBytecodes and 
> RewriteFrequentPairs are disabled due to ConstantMethod in CDS are 
> mapped read only. So memory fault will be triggered when 
> RewriteBytecodes turned on. This also disable all method rewritten, 
> leads interpreter run slower. Observed about 2% regression with C2 on 
> some benchmarks, since interpreter speed is important to C2. By enable 
> RewriteBytecodes and RewriteFrequentPairs under CDS enabled, adding 
> _nofast_xxxx for corresponding fast codes at dump time to avoid byte 
> code rewritten at run time, we prevent byte code rewritten and modify 
> the read only shared portion in CDS. Meanwhile other byte codes with 
> fast codes still get speed up.
>
> Tests: JPRT, jtreg, refworkload (20+ benchmarks) on all supported 
> platforms.  Interpreter only tests showed about 3% improvement.

What performance did you measure?  Is it -Xint -Xshare:on with and 
without your patch?  It was only 3% better?

What was the difference in performance with -Xint -XX:-RewriteBytecodes 
vs. -Xint -XX:+RewriteBytecodes/FrequentPairs? I thought this was around 
15%.

Thanks,
Coleen

>
> Thanks
> Yumin
>



More information about the hotspot-runtime-dev mailing list