Hi Mandy,
On 16/02/2016 6:05 AM, Mandy Chung wrote:
On Feb 15, 2016, at 9:06 AM, Uwe Schindler <uschind...@apache.org> wrote:
Hi,
On 15/02/2016 14:57, Chris Hegarty wrote:
Peter,
http://cr.openjdk.java.net/~plevart/jdk9-dev/removeInternalCleaner/webrev.02/
This patch looks correct to me. The innocuous thread created for common
cleaner is of Thread.MAX_PRIORITY - 2 priority whereas the reference handler
thread is of MAX_PRIORITY priority. I don’t know if this would impact any
real-world application or whether worth having a dedicated thread with
MAX_PRIORTY (David Holmes may have recommendation to this).
How did you know I would read this? :)
Thread priority has no meaning on Solaris, Linux or BSD/OSX by default.
Only Windows actually applies thread priorities under normal conditions.
The difference between MAX_PRIORITY and MAX_PRIORITY-2 is the former
uses THREAD_PRIORITY_HIGHEST and the latter
THREAD_PRIORITY_ABOVE_NORMAL. Without any real/reasonable workloads for
benchmarking it is all somewhat arbitrary. Arguably the Reference
handler thread has more work to do in general so might be better given
the higher priority.
Cheers,
David
-----
Another subtle difference is no more package access check whether running with
security manager. On the other hand, “suppressAccessChecks” permission is
still checked. I don’t have issue with this.
Given the changes in 6857566, I think your proposal to remove
jdk.internal.ref.Cleaner should be good. ( Note: third party libraries
digging into internals of direct buffers will need to change again ).
I think so too, the main think is that the fast path for cleaners has
been removed (and I see that it has).
What do you mean with fast path?
I believe Alan refers to the fast path handling jdk.internal.ref.Cleaner via
the Reference Handler thread.
What this means to existing code is that the java.nio.DirectByteBuffer::cleaner
method still exists but with java.lang.ref.Cleaner.Cleanable return type. You
still need to call setAccessible(true) on the cleaner method. You can then
statically reference java.lang.ref.Cleaner.Cleanable::clean method
Mandy
The usual code pattern for forceful unmapping did not change:
- Make java.nio.DirectByteBuffer#cleaner() accessible (this is the thing where
you need to fight against the Java access checks).
- call clean() (Runnable#run() in previous commit) on the returned object (with
correct casting to interface before calling, this is where existing legacy code
must change).
In preparation for these changes, I made a patch for Lucene (Solr/Elasticsearch) to
"compile" the whole unmapping logic into a MethodHandle (this allows me to do
all needed accesibility/security checks early on startup and before actually invoking
unmapping). This allows us to tell user that forceful unmapping works *before* we
actually need to call it:
https://issues.apache.org/jira/secure/attachment/12787878/LUCENE-6989.patch
This will be part of Lucene 6.0 which will require Java 8 as minimum. Earlier Lucene
version will not be compatible to Java 9 and can only be used in "slower"
non-mmap mode.
Thanks,
Uwe