Hi Uwe,
On 12/11/2016 01:57 PM, Uwe Schindler wrote:
Hi,
How about the following:
-Check that the buffer is direct, if not throw IAE(“not direct buffer”)
-Check that buffer has attachment==null (this tells you that it’s not
a slice/dup), if not throw IAE(“not allowed to free duplicates/slices”)
-Finally do the standard if (cleaner!=null) cleaner.clean(), but don’t
throw any exceptions if cleaner is null (as this is implementation detail)
This allows for empty buffers without cleaner that are still marked as
direct. But it disallows all slices or duplicates.
Yes, this would be the right logic I agree. It would silently ignore the
requests to free memory for buffers constructed via JNI's
NewDirectByteBuffer calls, but I suppose this would not be a problem in
practice.
I am fine with Alan’s proposal to restrict to MappedByteBuffer but
that’s out of my interest – I am happy to unmap mapped byte buffers. I
would also place the method in the legacy sun.misc.Unsafe only, the
JDK-internal private one is not accessible to the outside. Of course
for consistency it could be in both, but primarily it must be in
sun.misc.Unsafe – that’s also what most code is using anyways.
Yes, internally, at least in java.base, the code can always directly
invoke the DirectBuffer's and Cleaner's methods...
Regards, Peter
Uwe
*From:*Peter Levart [mailto:peter.lev...@gmail.com]
*Sent:* Saturday, December 10, 2016 10:23 PM
*To:* Uwe Schindler <uschind...@apache.org>; Chris Hegarty
<chris.hega...@oracle.com>
*Cc:* jigsaw-dev@openjdk.java.net; Core-Libs-Dev
<core-libs-...@openjdk.java.net>
*Subject:* Re: Java 9 build 148 causes trouble in Apache
Lucene/Solr/Elasticsearch
On 12/10/2016 09:00 PM, Uwe Schindler wrote:
Hi,
We noticed that buffers with zero length also have no cleaner.
This is why we also have the null check in our code (see Github)
and the guardWithTest in the MethodHandle, although we never free
duplicates. So a noop is better imho.
Oh yes, good catch. Then what about being noop just for zero length? I
don't know, maybe I'm just being paranoid and those who would use this
API know perfectly well what they are doing. I'm just imagining a
situation where one would create and keep just a duplicate of a direct
buffer and afterwards use it to try to deallocate the native memory.
This would be a noop, but the developer would think it works as GC
would finally do it for him. I think it's better to throw an exception
to prevent such situations...
Regards, Peter
I like the Unsafe approach. To me both variants are fine.
Uwe
Am 10. Dezember 2016 20:47:46 MEZ schrieb Peter Levart
<peter.lev...@gmail.com> <mailto:peter.lev...@gmail.com>:
Hi Chris,
On 12/10/2016 06:11 PM, Chris Hegarty wrote:
How about: Unsafe::deallocate(ByteBuffer directBuffer)?
http://cr.openjdk.java.net/~chegar/Unsafe_deallocate/
<http://cr.openjdk.java.net/%7Echegar/Unsafe_deallocate/>
Apart from the fact that Unsafe is (was?) reserved for
low-level stuff, I think this approach is reasonable. Is the
method in jdk.internal.misc.Unsafe needed? You could add the
method just to the sun.misc.Unsafe (to keep internal Unsafe
free from hacks) and export the two packages selectively to
jdk.unsupported.
We could attempt to limit this to the direct buffer that "owns" the
memory, i.e. not a duplicate or a slice, but I'm not sure it is
worth
it.
What you have here *is* limited to direct ByteBuffer(s) that
"own" the memory. Derived buffer(s) (duplicated or sliced) do
not have a Cleaner instance (they have an 'attachment' to keep
the 1st-level buffer reachable while they are reachable). I
would even make it more unforgiving by throwing an IAE if the
passed-in buffer didn't have a Cleaner. In addition I would
specify this behavior. For example:
"Deallocates the underlying memory associated with given
directBuffer if the buffer was obtained from either {@link
ByteBuffer#allocateDirect} or {@link FileChannel#map} methods.
In any other case (when the buffer is not a direct buffer or
was obtained by {@link ByteBuffer#duplicate() duplicating} or
{@link ByteBuffer#slice(int, int) slicing} a direct buffer),
the method throws {@code IllegalArgumentException}.
Regards, Peter
