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

[Roman Kennke]

Hi Vladimir,

Oh! Now that is surprising, and weird! I will have to discuss this with
Roland and likely open a new bug for this.

Thanks for figuring this out!

Please carry on with your changes then.

Thanks,
Roman

> 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
>>>>
>>>
>