JDK-8221355 : Performance regression after JDK-8155635 backport into 8u
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 8u202
  • Priority: P2
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2019-03-23
  • Updated: 2019-08-14
  • Resolved: 2019-04-02
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 8 Other
8u212Fixed openjdk8u212Fixed
Related Reports
Relates :  
JDK-8181822 - C2: Mixed unsafe oop accesses break alias analysis
Relates :  
JDK-8155635 - C2: Mixed unsafe oop accesses break alias analysis
Sub Tasks
JDK-8222206 :  
Release Note: Possible Performance Regression in JDK 8 Updates 202, 211, and 212 - Closed
Description
Tom Rodriguez wrote in a comment on JDK-8181822:

Why is the version of this fix for 8 different than what's in 9 and later? 

// Can base be NULL? Otherwise, always on-heap access. 
   bool can_access_non_heap = TypePtr::NULL_PTR->higher_equal(_gvn.type(heap_base_oop)); 
+ if (can_access_non_heap && type == T_OBJECT) { 
+ return false; // off-heap oop accesses are not supported 
+ } 

means that if the base pointer hasn't been null checked before the Unsafe.getObject then it won't be intrinsified at all. 9 and later don't have this check. http://hg.openjdk.java.net/jdk9/jdk9/hotspot/file/b756e7a2ec33/src/share/vm/opto/library_call.cpp#l2324 We see a huge slowdown in some code using Unsafe for reflection as a result of this change.

I attached a microbenchmark showing the difference. On 8u192, 9 and 11 the loop runs in about 40ms but with 8u202 it takes about 3000ms.
Comments
RFR: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2019-April/033376.html
10-04-2019
Reformatted patch that can be applied to current jdk8u/jdk8u-dev: http://cr.openjdk.java.net/~shade/8221355/8221355-01.patch Benchmark: http://cr.openjdk.java.net/~shade/8221355/UnsafeGetObjectBench.java http://cr.openjdk.java.net/~shade/8221355/benchmarks.jar Benchmark Mode Cnt Score Error Units # current jdk8u/jdk8u-dev UnsafeGetObjectBench.test_null_checked avgt 5 2.784 �� 0.014 ns/op UnsafeGetObjectBench.test_plain avgt 5 14.480 �� 0.070 ns/op # current jdk8u/jdk8u-dev + fix UnsafeGetObjectBench.test_null_checked avgt 5 2.793 �� 0.049 ns/op UnsafeGetObjectBench.test_plain avgt 5 2.792 �� 0.067 ns/op This also shows the workaround: null-checking the receiver argument recovers the regression. Perfasm shows that Unsafe.getObject is indeed not intrinsified in broken case, and is intrinsified in fixed: http://cr.openjdk.java.net/~shade/8221355/8u-broken.perfasm http://cr.openjdk.java.net/~shade/8221355/8u-fixed.perfasm
10-04-2019
Aha, I think the fix that David posted above is the final one.
10-04-2019
Is it possible to see the final fix for this issue? Since the original regression went into public jdk8u, it should also be fixed there.
10-04-2019
fix should look something like this: --- a/src/share/vm/opto/library_call.cpp Mon Mar 25 13:32:16 2019 +0000 +++ b/src/share/vm/opto/library_call.cpp Mon Apr 01 21:07:24 2019 -0400 @@ -2608,9 +2608,10 @@ Node* offset = top(); Node* val; + // The base is either a Java object or a value produced by Unsafe.staticFieldBase + Node* base = argument(1); // type: oop + if (!is_native_ptr) { - // The base is either a Java object or a value produced by Unsafe.staticFieldBase - Node* base = argument(1); // type: oop // The offset is a value produced by Unsafe.staticFieldOffset or Unsafe.objectFieldOffset offset = argument(2); // type: long // We currently rely on the cookies produced by Unsafe.xxxFieldOffset @@ -2630,11 +2631,12 @@ val = is_store ? argument(3) : NULL; } + if ((_gvn.type(base)->isa_ptr() == TypePtr::NULL_PTR) && type == T_OBJECT) { + return false; // off-heap oop accesses are not supported + } + // Can base be NULL? Otherwise, always on-heap access. bool can_access_non_heap = TypePtr::NULL_PTR->higher_equal(_gvn.type(heap_base_oop)); - if (can_access_non_heap && type == T_OBJECT) { - return false; // off-heap oop accesses are not supported - } const TypePtr *adr_type = _gvn.type(adr)->isa_ptr();
02-04-2019