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


Reply via email to