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