Hi Uwe,
On 12/10/2016 01:33 PM, Uwe Schindler wrote:
Hi Peter,
this would be a great fix! Thanks!!!
I also think the non-static method is superior to my original
proposal, because it allows us to do the security check **once**,
which is really needed for Lucene. I am still fine if the permission
is still checked on every unmapping, but we need to do the check
up-front. If you look at our current unmapping code
(https://goo.gl/TfQWl6), you will the that the detector checks for the
extra runtime permission upfront, so we can be sure that the actual
unmapping will work for sure. This is also the reason why we use
MethodHandles: As those are compiled on investigation of possible
unmapping variants depending on the VM, we can “compile” the
MethodHandle and later call it as often as we like, without the risk
that it breaks for incompatibility reasons. The MethodHandle makes
sure that all types are checked up front.
About MappedByteBuffer vs ByteBuffer (or maybe just
java.nio.Buffer!?): I’d make it generic so it works with any direct
buffer (maybe also non-byte ones). For Lucene it does not matter, but
other projects (I know Cassandra or other off-Heap frameworks) do the
same with buffers that were allocated direct (not only mmapped). The
method signature in your proposal is also compatible to our
requirements: We can create the DirectBufferDeallocator up front and
then produce a MH which is bound to the allocator.
I choose to limit the method to ByteBuffer type because this is the
public static type used in programs for instances that are possibly
"owning" the underlying native memory. Other-typed buffers or even
2nd-level direct ByteBuffers obtained by duplicating or slicing are just
views and do not "own" the underlying memory. While it would be possible
to trigger deallocation / unmapping via any buffer that references the
owning buffer, I think this might be prone to bugs. By limiting the
method to 1st-level direct ByteBuffer(s), the programmer is forced to
think about ownership and lifetime of derived buffers and consequently
write better code.
So on 2nd thought, the API might be even better to reject non-direct and
2nd-level direct ByteBuffer(s) by throwing an exception rather than
silently ignoring the deallocation request.
I will make a pull request to Lucene using your current proposal so
you have a “patch” to test this with Lucene before you commit
something like this.
Let us first wait for a proposal from Oracle to see what they have in
mind...
Regards, Peter
Uwe
-----
Uwe Schindler
uschind...@apache.org
ASF Member, Apache Lucene PMC / Committer
Bremen, Germany
http://lucene.apache.org/
*From:*Peter Levart [mailto:peter.lev...@gmail.com]
*Sent:* Saturday, December 10, 2016 12:10 PM
*To:* Alan Bateman <alan.bate...@oracle.com>; Uwe Schindler
<uschind...@apache.org>; jigsaw-...@openjdk.java.net; Core-Libs-Dev
<core-libs-dev@openjdk.java.net>
*Subject:* Re: Java 9 build 148 causes trouble in Apache
Lucene/Solr/Elasticsearch
Hi,
On 12/10/2016 06:14 AM, Alan Bateman wrote:
On 09/12/2016 22:32, Uwe Schindler wrote:
Hi,
I updated our Jenkins server for the JDK 9 preview testing to
use build 148. Previously we had build 140 and build 147,
which both worked without any issues. But after the update the
following stuff goes wrong:
(1) Unmapping of direct buffers no longer works, although this
API was marked as critical because there is no replacement up
to now, so code can unmap memory mapped files, which is one of
the most important things Apache Lucene needs to use to access
huge random access files while reading the index. Without
memory mapping, the slowdown for Lucene users will be huge
sun.misc.Cleaner was indeed on the original list of APIs for JEP
260 to identify as a "critical internal API". It turned out not to
be useful because it would have required some way to get the
Cleaner in the first place. That lead to the "new" hack that is
reading the private "cleaner" field from DBB and treating it as a
Runnable. That hack now breaks because setAccessible has changed
in jdk-9+148 to align with the JSR 376 proposal tracked as
#AwkwardStrongEncapsulation.
No need to panic though, there is an update to JEP 260 coming soon
for this specific need. Details TDB but it will probably be a
method in jdk.unsupported module. It does mean that libraries
using the old (or "new") hacks will need to change. I hope it will
be seen as a reasonable compromise for this generally awkward issue.
-Alan
Something like the following?
http://cr.openjdk.java.net/~plevart/jdk9-dev/DirectBufferDeallocator/webrev.01/
<http://cr.openjdk.java.net/%7Eplevart/jdk9-dev/DirectBufferDeallocator/webrev.01/>
Regards, Peter
