[PATCH] Reduce Chance Of Mistakenly Early Backing Memory Cleanup

Peter Levart peter.levart at gmail.com
Sat Feb 3 01:04:41 UTC 2018


Hi,

I have one question, maybe stupid. I'm wondering about one situation. 
Suppose you have this Java code:

void m() {
     // code before...

     Objects.requireNonNull(this);
}

Of course, the non-null check will never throw NPE. The check will most 
likely even be optimized away by JIT. But is JIT allowed to optimize it 
away so that the "code before" observes the effects of 'this' being 
unreachable? Wouldn't that violate the semantics of the program as it 
would 1st observe that some object is not reachable and after that it 
would observer that 'this' still points to some object which by 
definition must be it?

If JIT is not allowed to optimize the null check so that the object 
becomes unreachable, then Objects.requireNonNull could be used as a 
replacement for Reference.reachabilityFence. It might even perform better.

What do you think?

Regards, Peter


On 02/01/18 23:50, Paul Sandoz wrote:
> Hi Ben,
>
> I don’t think you require the fence in all the cases you have currently placed it e.g. here for example
>
>          $memtype$ y = $toBits$(x);
>          UNSAFE.put$Memtype$Unaligned(null, a, y, bigEndian);
> +        Reference.reachabilityFence(this);
>          return this;
>
> since “this” is being returned it will be kept live during the unsafe access.
>
> Would you mind providing a JMH benchmark and results for the performance with and without the fence for say a put and/or a get. I would like to understand the performance impact on HotSpot, this is one reason why we have yet to add such fences as it will likely impact performance.
>
> At the moment we are relying on the method not being inlined, which is an expedient technique to make it functional and keep a reference alive but not necessarily optimal for usages in DBB.
>
> For more details see:
>
>    https://bugs.openjdk.java.net/browse/JDK-8169605 <https://bugs.openjdk.java.net/browse/JDK-8169605>
>    https://bugs.openjdk.java.net/browse/JDK-8149610 <https://bugs.openjdk.java.net/browse/JDK-8149610>
>
> Thanks,
> Paul.
>
>> On Feb 1, 2018, at 6:55 AM, Ben Walsh <ben_walsh at uk.ibm.com> wrote:
>>
>> This contribution forms a partial solution to the problem detailed here -
>> http://thevirtualmachinist.blogspot.ca/2011/07/subtle-issue-of-reachability.html
>> .
>>
>> In this context, this contribution provides "markers" such that a suitably
>> "aware" compiler can reduce the chances of such a problem occurring with
>> each of the Direct<Type>Buffer<RW><BO> objects and MappedByteBuffer
>> object. The reachabilityFences may prevent crashes / exceptions due to
>> cleaning up the backing memory before the user has finished using the
>> pointer.
>>
>> Any compiler not suitably "aware" could be modified to make use of the
>> "markers".
>>
>>
>> I would like to pair with a sponsor to contribute this patch ...
>>
>> -------------------------------------------------------------------------------------------------------------------
>> diff -r d51e64840b4f
>> src/java.base/share/classes/java/nio/Direct-X-Buffer-bin.java.template
>> ---
>> a/src/java.base/share/classes/java/nio/Direct-X-Buffer-bin.java.template
>> Wed Jan 31 12:04:53 2018 +0800
>> +++
>> b/src/java.base/share/classes/java/nio/Direct-X-Buffer-bin.java.template
>> Thu Feb 01 11:30:10 2018 +0000
>> @@ -1,5 +1,5 @@
>> /*
>> - * Copyright (c) 2000, 2016, Oracle and/or its affiliates. All rights
>> reserved.
>> + * Copyright (c) 2000, 2018, Oracle and/or its affiliates. All rights
>> reserved.
>>   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>   *
>>   * This code is free software; you can redistribute it and/or modify it
>> @@ -33,15 +33,21 @@
>>
>>      private $type$ get$Type$(long a) {
>>          $memtype$ x = UNSAFE.get$Memtype$Unaligned(null, a, bigEndian);
>> -        return $fromBits$(x);
>> +        $type$ y = $fromBits$(x);
>> +        Reference.reachabilityFence(this);
>> +        return y;
>>      }
>>
>>      public $type$ get$Type$() {
>> -        return get$Type$(ix(nextGetIndex($BYTES_PER_VALUE$)));
>> +        $type$ y = get$Type$(ix(nextGetIndex($BYTES_PER_VALUE$)));
>> +        Reference.reachabilityFence(this);
>> +        return y;
>>      }
>>
>>      public $type$ get$Type$(int i) {
>> -        return get$Type$(ix(checkIndex(i, $BYTES_PER_VALUE$)));
>> +        $type$ y = get$Type$(ix(checkIndex(i, $BYTES_PER_VALUE$)));
>> +        Reference.reachabilityFence(this);
>> +        return y;
>>      }
>>
>> #end[rw]
>> @@ -50,6 +56,7 @@
>> #if[rw]
>>          $memtype$ y = $toBits$(x);
>>          UNSAFE.put$Memtype$Unaligned(null, a, y, bigEndian);
>> +        Reference.reachabilityFence(this);
>>          return this;
>> #else[rw]
>>          throw new ReadOnlyBufferException();
>> diff -r d51e64840b4f
>> src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
>> --- a/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
>> Wed Jan 31 12:04:53 2018 +0800
>> +++ b/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
>> Thu Feb 01 11:30:10 2018 +0000
>> @@ -1,5 +1,5 @@
>> /*
>> - * Copyright (c) 2000, 2016, Oracle and/or its affiliates. All rights
>> reserved.
>> + * Copyright (c) 2000, 2018, Oracle and/or its affiliates. All rights
>> reserved.
>>   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>   *
>>   * This code is free software; you can redistribute it and/or modify it
>> @@ -28,6 +28,7 @@
>> package java.nio;
>>
>> import java.io.FileDescriptor;
>> +import java.lang.ref.Reference;
>> import jdk.internal.misc.VM;
>> import jdk.internal.ref.Cleaner;
>> import sun.nio.ch.DirectBuffer;
>> @@ -312,6 +313,7 @@
>>      public $Type$Buffer put($type$ x) {
>> #if[rw]
>>          UNSAFE.put$Swaptype$(ix(nextPutIndex()), $swap$($toBits$(x)));
>> +        Reference.reachabilityFence(this);
>>          return this;
>> #else[rw]
>>          throw new ReadOnlyBufferException();
>> @@ -321,6 +323,7 @@
>>      public $Type$Buffer put(int i, $type$ x) {
>> #if[rw]
>>          UNSAFE.put$Swaptype$(ix(checkIndex(i)), $swap$($toBits$(x)));
>> +        Reference.reachabilityFence(this);
>>          return this;
>> #else[rw]
>>          throw new ReadOnlyBufferException();
>> @@ -347,6 +350,7 @@
>>              if (srem > rem)
>>                  throw new BufferOverflowException();
>>              UNSAFE.copyMemory(sb.ix(spos), ix(pos), (long)srem <<
>> $LG_BYTES_PER_VALUE$);
>> +            Reference.reachabilityFence(this);
>>              sb.position(spos + srem);
>>              position(pos + srem);
>>          } else if (src.hb != null) {
>> @@ -413,6 +417,7 @@
>>          int rem = (pos <= lim ? lim - pos : 0);
>>
>>          UNSAFE.copyMemory(ix(pos), ix(0), (long)rem <<
>> $LG_BYTES_PER_VALUE$);
>> +        Reference.reachabilityFence(this);
>>          position(rem);
>>          limit(capacity());
>>          discardMark();
>> @@ -505,6 +510,7 @@
>>      void _put(int i, byte b) {                  // package-private
>> #if[rw]
>>          UNSAFE.putByte(address + i, b);
>> +        Reference.reachabilityFence(this);
>> #else[rw]
>>          throw new ReadOnlyBufferException();
>> #end[rw]
>> diff -r d51e64840b4f
>> src/java.base/share/classes/java/nio/MappedByteBuffer.java
>> --- a/src/java.base/share/classes/java/nio/MappedByteBuffer.java Wed Jan
>> 31 12:04:53 2018 +0800
>> +++ b/src/java.base/share/classes/java/nio/MappedByteBuffer.java Thu Feb
>> 01 11:30:10 2018 +0000
>> @@ -1,5 +1,5 @@
>> /*
>> - * Copyright (c) 2000, 2013, Oracle and/or its affiliates. All rights
>> reserved.
>> + * Copyright (c) 2000, 2018, Oracle and/or its affiliates. All rights
>> reserved.
>>   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>   *
>>   * This code is free software; you can redistribute it and/or modify it
>> @@ -26,6 +26,7 @@
>> package java.nio;
>>
>> import java.io.FileDescriptor;
>> +import java.lang.ref.Reference;
>> import jdk.internal.misc.Unsafe;
>>
>>
>> @@ -164,6 +165,7 @@
>>          // is computed as we go along to prevent the compiler from
>> otherwise
>>          // considering the loop as dead code.
>>          Unsafe unsafe = Unsafe.getUnsafe();
>> +        Reference.reachabilityFence(this);
>>          int ps = Bits.pageSize();
>>          int count = Bits.pageCount(length);
>>          long a = mappingAddress(offset);
>> -------------------------------------------------------------------------------------------------------------------
>>
>>
>> Regards,
>> Ben Walsh
>>
>>
>> Unless stated otherwise above:
>> IBM United Kingdom Limited - Registered in England and Wales with number
>> 741598.
>> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
>>



More information about the core-libs-dev mailing list