I think it best to keep whatever we do here simple and straight forward. It is, after all, just a stop gap until a public API can be put in place in a future release ( which I believe is a realistic possibility given some of the discussions that I have heard about, but that is for another day).
If we add a method to Unsafe, then we get the benefit of being protected by the security manager for free ( you need reflective access to get "theUnsafe" field). Most of the code I've seen in this area delves into Unsafe anyway. How about: Unsafe::deallocate(ByteBuffer directBuffer)? http://cr.openjdk.java.net/~chegar/Unsafe_deallocate/ 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. Once we reach agreement on the technical solution, I will add appropriate wording to JEP 260 to cover this case. -Chris. > On 10 Dec 2016, at 13:08, Peter Levart <peter.lev...@gmail.com> wrote: > > 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 >> >
