RE: [8u] Request for enhancement backport approval for 8144019

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RE: [8u] Request for enhancement backport approval for 8144019

Doerr, Martin
Hi Michihiro,

there are still shared code parts which should not be backported with this change. I have a couple of requests and suggestions.

c1_Compilation.hpp
I think it would be better to change #ifndef PPC to #ifndef PPC32 as it restricts the size unnecessarily for PPC64. In addition, the change will be PPC64 only if you remove the type cast.

c1_GraphBuilder.cpp, globals.hpp
I think AlwaysSafeConstructors should not get introduced in jdk8u.

c1_LIRGenerator.hpp, c1_RangeCheckElimination.hpp
Delete operators need to be changed to fix slow debug build on AIX (was done by a later change):
  void operator delete(void* p) { ShouldNotReachHere(); }
  void operator delete[](void* p) { ShouldNotReachHere(); }

c1_Runtime1.cpp
I think the PPC32 code should not get removed. Please use #ifdef PPC32 instead?

deoptimization.cpp, deoptimization.hpp
I think the exec_mode change should not be backported as it is a part of a jdk9 feature.

As this is a major feature backport, it will motivate us to accept this change if we can get a commitment from your side that you maintain the PPC64 part of the change until end of life of jdk8u.
That includes testing on all PPC64 platforms and making sure future critical bug fixes get applied.

Best regards,
Martin


From: Michihiro Horie [mailto:[hidden email]]
Sent: Dienstag, 18. April 2017 05:25
To: [hidden email]
Cc: Hiroshi H Horii <[hidden email]>; Tim Ellison <[hidden email]>; Gustavo Bueno Romero <[hidden email]>; Doerr, Martin <[hidden email]>; Simonis, Volker <[hidden email]>; [hidden email]
Subject: [8u] Request for enhancement backport approval for 8144019

Hello,
(I resent this message because my previous one was not listed in the hotspot-dev archive.)

Would you please approve the backport of the following ppc64 change to jdk8u-dev?
Since this backport includes changes in shared code, we would like hotspot team to verify that the changes will have no impact on x86. We have already confirmed these changes have no impact on x86 with jtreg.

8144019: PPC64 C1: Introduce Client Compiler
Bug: https://bugs.openjdk.java.net/browse/JDK-8144019
Webrev: http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/4a24de859a87
Review: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2015-December/020229.html
URL: http://hg.openjdk.java.net/jdk9/hs/hotspot/rev/4a24de859a87


[Webrev for backport to 8u]
http://cr.openjdk.java.net/~horii/jdk8u_support_tiered_ppc64le/hotspot/webrev02<http://cr.openjdk.java.net/%7Ehorii/jdk8u_support_tiered_ppc64le/hotspot/webrev02>/
http://cr.openjdk.java.net/~horii/jdk8u_support_tiered_ppc64le/jdk/webrev02<http://cr.openjdk.java.net/%7Ehorii/jdk8u_support_tiered_ppc64le/jdk/webrev02>/


[A rationale for why the enhancement should be backported]
C1 and Tiered Compilation provide faster startup for JVMs. In recent popular workloads, such as Hadoop and Spark (via Yarn), multiple JVMs concurrently run with their short life cycles. C1 and Tiered Compilation significantly shorten execution time of these workloads. For example, the above changes provide 15% improvement of execution time of Spark (2.1.0) q16 on POWER8 3.325MHz with 1.0 scale factor.


[Tests]
We observed the same test results in jtreg with/without above changes on x86, ppc64, and ppc64le.
Also, we have run SPECjbb2015, Spark TPC-DS queries, and DaCapo benchmark suite. We confirmed all benchmarks worked well.


[Risks]
Most of changes in shared codes have effects on only ppc64 and ppc64le.

src/share/vm/c1/c1_Compilation.hpp: 1 line changed: 0 ins; 0 del; 1 mod; 340 unchg
src/share/vm/c1/c1_GraphBuilder.cpp: 10 lines changed: 9 ins; 0 del; 1 mod; 4460 unchg
src/share/vm/c1/c1_IR.cpp: 2 lines changed: 1 ins; 0 del; 1 mod; 1404 unchg
src/share/vm/c1/c1_IR.hpp: 7 lines changed: 6 ins; 0 del; 1 mod; 357 unchg
src/share/vm/c1/c1_LIR.hpp: 2 lines changed: 0 ins; 0 del; 2 mod; 2506 unchg
src/share/vm/c1/c1_LIRGenerator.cpp: 10 lines changed: 8 ins; 0 del; 2 mod; 3673 unchg
src/share/vm/c1/c1_LIRGenerator.hpp: 6 lines changed: 5 ins; 0 del; 1 mod; 644 unchg
src/share/vm/c1/c1_RangeCheckElimination.hpp: 7 lines changed: 6 ins; 0 del; 1 mod; 243 unchg
src/share/vm/c1/c1_Runtime1.cpp: 8 lines changed: 0 ins; 7 del; 1 mod; 1483 unchg
src/share/vm/runtime/deoptimization.cpp: 10 lines changed: 2 ins; 0 del; 8 mod; 2074 unchg
src/share/vm/runtime/deoptimization.hpp: 5 lines changed: 1 ins; 0 del; 4 mod; 385 unchg
src/share/vm/runtime/globals.hpp: 3 lines changed: 3 ins; 0 del; 0 mod; 4027 unchg

Changes in src/cpu/ppc will add C1 for ppc64 and ppc64le. We believe that impacts on Interpreter and C2 are limited on ppc64 and ppc64le.


Best regards,
--
Michihiro,
IBM Research - Tokyo

Loading...