[14] RFR (S): 8226411: C2: Avoid memory barriers around off-heap unsafe accesses

[Vladimir Ivanov]

Roman,

JDK-8220714 looks like a bug in Shenandoah barrier expansion.

I slightly modified the test to simplify the analysis [1].

While running the test [2] I'm seeing the following:

   127 LoadI  === 44   7 125
   160 StoreI === 44   7  91 127
    94 LoadI  === 44   7  91
   193 StoreI === 44 160 125  94

After expansion is over, it looks as follows:

   127 LoadI  ===  44 209 125
   160 StoreI ===  44 209  91 127
    94 LoadI  ===  44 160  91
   193 StoreI ===  44 160 125  94

Note that 94 LoadI depends on 160 StoreI memory now. Before the 
expansion they were independent (7 Parm == initial memory state).

And then 94 goes away, since it now reads updated value:

< 		< 94	LoadI	=== _ _ _  [[]]   [3200094]
 > int		 127	LoadI	===  44  209  125  [[ 160  193 ]]  @rawptr:BotPTR, 
idx=Raw; unsafe #int (does not depend only on test) !orig=[94] !jvms: 
Unsafe::getInt @ bci:3 TestUnsafeOffheapSwap$Memory::getInt @ bci:14 
TestUnsafeOffheapSwap$Memory::swap @ bci:10 
TestUnsafeOffheapSwap::testUnsafeHelper @ bci:7

The final graph is evidently wrong:

   127 LoadI  ===  44 209 125
   160 StoreI ===  44 209  91 127
   193 StoreI ===  44 160 125 127

Let me know how you want to proceed with it.

Best regards,
Vladimir Ivanov

[1]

diff --git 
a/test/hotspot/jtreg/gc/shenandoah/compiler/TestUnsafeOffheapSwap.java 
b/test/hotspot/jtreg/gc/shenandoah/compiler/TestUnsafeOffheapSwap.java
--- a/test/hotspot/jtreg/gc/shenandoah/compiler/TestUnsafeOffheapSwap.java
+++ b/test/hotspot/jtreg/gc/shenandoah/compiler/TestUnsafeOffheapSwap.java
@@ -57,6 +57,16 @@
          }
      }

+    static void testUnsafeHelper(int i) {
+        mem.swap(i - 1, i);
+    }
+
+    static void testArrayHelper(int[] arr, int i) {
+        int tmp = arr[i - 1];
+        arr[i - 1] = arr[i];
+        arr[i] = tmp;
+    }
+
      static void test() {
          Random rnd = new Random(SEED);
          for (int i = 0; i < SIZE; i++) {
@@ -72,10 +82,8 @@
          }

          for (int i = 1; i < SIZE; i++) {
-            mem.swap(i - 1, i);
-            int tmp = arr[i - 1];
-            arr[i - 1] = arr[i];
-            arr[i] = tmp;
+            testUnsafeHelper(i);
+            testArrayHelper(arr, i);
          }

          for (int i = 0; i < SIZE; i++) {

[2] $ java -cp 
JTwork/classes/gc/shenandoah/compiler/TestUnsafeOffheapSwap.d/ 
--add-exports=java.base/jdk.internal.misc=ALL-UNNAMED 
-XX:-UseOnStackReplacement -XX:-BackgroundCompilation 
-XX:-TieredCompilation  -XX:+UnlockExperimentalVMOptions 
-XX:+UseShenandoahGC -XX:+PrintCompilation -XX:CICompilerCount=1 
-XX:CompileCommand=quiet 
-XX:CompileCommand=compileonly,*::testUnsafeHelper 
-XX:CompileCommand=print,*::testUnsafeHelper -XX:PrintIdealGraphLevel=0 
-XX:-VerifyOops -XX:-UseCompressedOops TestUnsafeOffheapSwap


Best regards,
Vladimir Ivanov

On 29.11.2019 20:18, Vladimir Ivanov wrote:
> 
>> Does it affect this:
>> https://bugs.openjdk.java.net/browse/JDK-8220714
> 
> Good point, Roman. Proposed patch breaks the fix for JDK-8220714.
> 
> I'll investigate what happens there and come back with a revised fix.
> 
> Best regards,
> Vladimir Ivanov
> 
>>> http://cr.openjdk.java.net/~vlivanov/8226411/webrev.00/
>>> https://bugs.openjdk.java.net/browse/JDK-8226411
>>>
>>> There were a number of fixes in C2 support for unsafe accesses recently
>>> which led to additional memory barriers around them. It improved
>>> stability, but in some cases it was redundant. One of important use
>>> cases which regressed is off-heap accesses [1]. The barriers around them
>>> are redundant because they are serialized on raw memory and don't
>>> intersect with any on-heap accesses.
>>>
>>> Proposed fix skips memory barriers around unsafe accesses which are
>>> provably off-heap (base == NULL).
>>>
>>> It (almost completely) recovers performance on the microbenchmark
>>> provided in JDK-8224182 [1].
>>>
>>> Testing: tier1-6.
>>>
>>> Best regards,
>>> Vladimir Ivanov
>>>
>>> [1] https://bugs.openjdk.java.net/browse/JDK-8224182
>>>
>>