[PATCH] Reduce Chance Of Mistakenly Early Backing Memory Cleanup
Paul Sandoz
paul.sandoz at oracle.com
Thu Feb 1 22:50:23 UTC 2018
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