Java 9 build 148 causes trouble in Apache Lucene/Solr/Elasticsearch

[Peter Levart]

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
>
> uschindler at apache.org
>
> ASF Member, Apache Lucene PMC / Committer
>
> Bremen, Germany
>
> http://lucene.apache.org/
>
> *From:*Peter Levart [mailto:peter.levart at gmail.com]
> *Sent:* Saturday, December 10, 2016 12:10 PM
> *To:* Alan Bateman <Alan.Bateman at oracle.com>; Uwe Schindler 
> <uschindler at apache.org>; jigsaw-dev at openjdk.java.net; Core-Libs-Dev 
> <core-libs-dev at 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
>