Hi,I checked the behavior of Objects.requireNonNull(this) at appropriate place instead of Reference.reachabilityFence(this) and it does seem to work. For example in the following test (see method m()):
import java.util.Objects;
import java.util.concurrent.atomic.AtomicLong;
public class ReachabilityTest {
private static final AtomicLong seq = new AtomicLong();
private static final AtomicLong deallocatedHwm = new AtomicLong();
private static long allocate() {
return seq.incrementAndGet();
}
private static void deallocate(long address) {
deallocatedHwm.accumulateAndGet(address, Math::max);
}
private static void access(long address) {
if (deallocatedHwm.get() == address) {
throw new IllegalStateException(
"Address " + address + " has allready been deallocated");
}
}
private long address = allocate();
@SuppressWarnings("deprecation")
@Override
protected void finalize() throws Throwable {
deallocate(address);
address = 0;
}
void m() {
long a = address;
if (a != 0) {
System.gc();
try { Thread.sleep(1); } catch (InterruptedException e) {}
access(a);
// Reference.reachabilityFence(this);
Objects.requireNonNull(this);
}
}
static void test() {
new ReachabilityTest().m();
}
public static void main(String[] args) {
while (true) {
test();
}
}
}
...Objects.requireNonNull does prevent otherwise reliable provoked
failure, just like Reference.reachabilityFence.
As to the speed of optimized-away Objects.requireNonNull() vs. Reference.reachabilityFence() I tried the following benchmark:
package jdk.test;
import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
import org.openjdk.jmh.annotations.Fork;
import org.openjdk.jmh.annotations.Level;
import org.openjdk.jmh.annotations.Measurement;
import org.openjdk.jmh.annotations.Mode;
import org.openjdk.jmh.annotations.OutputTimeUnit;
import org.openjdk.jmh.annotations.Param;
import org.openjdk.jmh.annotations.Scope;
import org.openjdk.jmh.annotations.Setup;
import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.annotations.Warmup;
import java.lang.ref.Reference;
import java.util.Arrays;
import java.util.Objects;
import java.util.concurrent.TimeUnit;
@BenchmarkMode(Mode.AverageTime)
@Warmup(iterations = 5)
@Measurement(iterations = 10)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@Fork(1)
@State(Scope.Thread)
public class ReachabilityBench {
static class Buf0 {
final byte[] buf;
Buf0(int len) {
buf = new byte[len];
}
@SuppressWarnings("deprecation")
@Override
protected void finalize() throws Throwable {
Arrays.fill(buf, (byte) -1);
}
byte get(int i) {
byte[] b = buf;
// this might get finalized before accessing the b element
byte r = b[i];
return r;
}
}
static class Buf1 {
final byte[] buf;
Buf1(int len) {
buf = new byte[len];
}
@SuppressWarnings("deprecation")
@Override
protected void finalize() throws Throwable {
Arrays.fill(buf, (byte) -1);
}
byte get(int i) {
byte[] b = buf;
byte r = b[i];
Reference.reachabilityFence(this);
return r;
}
}
static class Buf2 {
final byte[] buf;
Buf2(int len) {
buf = new byte[len];
}
@SuppressWarnings("deprecation")
@Override
protected void finalize() throws Throwable {
Arrays.fill(buf, (byte) -1);
}
byte get(int i) {
byte[] b = buf;
byte r = b[i];
Objects.requireNonNull(this);
return r;
}
}
Buf0 buf0;
Buf1 buf1;
Buf2 buf2;
@Param({"64"})
public int len;
@Setup(Level.Trial)
public void setup() {
buf0 = new Buf0(len);
buf1 = new Buf1(len);
buf2 = new Buf2(len);
}
@Benchmark
public int sumBuf0() {
int s = 0;
for (int i = 0; i < len; i++) {
s += buf0.get(i);
}
return s;
}
@Benchmark
public int sumBuf1() {
int s = 0;
for (int i = 0; i < len; i++) {
s += buf1.get(i);
}
return s;
}
@Benchmark
public int sumBuf2() {
int s = 0;
for (int i = 0; i < len; i++) {
s += buf2.get(i);
}
return s;
}
}
The results are as follows:
Benchmark (len) Mode Cnt Score Error Units
ReachabilityBench.sumBuf0 64 avgt 10 20.530 ± 0.052 ns/op
ReachabilityBench.sumBuf1 64 avgt 10 184.402 ± 0.531 ns/op
ReachabilityBench.sumBuf2 64 avgt 10 20.502 ± 0.027 ns/op
Objects.requireNonNull() shows zero overhead here.
I guess the main question is whether Objects.requireNonNull(this)
behavior in the former test is a result of chance and current Hotspot
behavior or is it somehow guaranteed by the spec.
Regards, Peter On 02/03/2018 02:04 AM, Peter Levart wrote:
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_wa...@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 witheach 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.templateWed Jan 31 12:04:53 2018 +0800 +++b/src/java.base/share/classes/java/nio/Direct-X-Buffer-bin.java.templateThu 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.templateWed Jan 31 12:04:53 2018 +0800+++ b/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.templateThu 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 Jan31 12:04:53 2018 +0800+++ b/src/java.base/share/classes/java/nio/MappedByteBuffer.java Thu Feb01 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 number741598.Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
