We don't need jdk.internal.ref.Cleaner any more

[Chris Hegarty]

Peter,

 > 
http://cr.openjdk.java.net/~plevart/jdk9-dev/removeInternalCleaner/webrev.02/

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 ).

-Chris.

On 09/02/16 13:35, Chris Hegarty wrote:
> Peter,
>
> On 08/02/16 10:58, Peter Levart wrote:
>
>> On 02/08/2016 10:33 AM, Chris Hegarty wrote:
>>> On 8 Feb 2016, at 06:27, Alan Bateman<Alan.Bateman at oracle.com>  wrote:
>>>
>>>> On 07/02/2016 22:20, Peter Levart wrote:
>>>>> :
>>>>>
>>>>> If the decision to remove sun.misc.Cleaner was partly influenced by
>>>>> the desire to maintain just 2 instead of 3 Cleaner(s), then my
>>>>> proposal to migrate JDK code to the public API might enable Oracle
>>>>> to reconsider keeping sun.misc.Cleaner.
>>>> The main issue driving this is the design principles that we have
>>>> listed in JEP 200. We don't want a standard module (java.base in
>>>> this case) exporting a non-standard API. This means surgery to
>>>> jettison the sun.misc package from java.base and move it to its own
>>>> module (jdk.unsupported is the proposal in JEP 260). This is painful
>>>> of course but we are at least in good shape for the most critical
>>>> internal API, sun.misc.Unsafe.
>>>>
>>>> For sun.misc.Cleaner then the original proposal was for it to be a
>>>> critical internal API too but it become clear that changing the type
>>>> of internal/private fields in the NIO buffer and channel classes
>>>> would break libraries that have been hacking into those fields. If
>>>> they are changing away then there seems little motive to keep
>>>> sun.misc.Cleaner so Chris moved into into jdk.internal.ref for now.
>>>> As to whether we even need to keep jdk.internal.ref.Cleaner then I
>>>> think the only remaining question was whether the special handling
>>>> of Cleaner in the Reference implementation was worth it or not. It
>>>> may not be, in which case your current proposal to just remove seems
>>>> right.
>>> Alan’s summary of the situation is spot on.
>>>
>>> Before moving sun.misc.Cleaner to jdk.internal.ref, I did enquire if
>>> it would be
>>> possible to just remove it and use the new public Cleaner, but I got
>>> feedback
>>> that there were some issues with failing NIO tests ( it appeared that
>>> Cleaners
>>> were not running as quickly as possible ). I didn’t look further into
>>> that at the
>>> time, since the VM still had special knowledge of these cleaners, but
>>> as you
>>> say this is now removed.  It is probably a good time to revisit this.
>>>
>>> -Chris.
>>
>> Hi Chris,
>>
>> Are you referring to the following test:
>>
>>    test/java/nio/Buffer/DirectBufferAllocTest.java ?
>>
>> This test was written specifically for the following issue:
>>
>> https://bugs.openjdk.java.net/browse/JDK-6857566
>>
>> ...which exercises multi-threaded allocation of direct buffers. Even
>> sun.misc.Cleaner was not quick enough to promptly deallocate them, so
>> allocation path was patched to help ReferenceHandler thread while
>> re-attempting to allocate new direct buffers. Transitioning to
>> java.lang.ref.Cleaner which uses additional thread to process
>> Cleanable(s), direct-buffer allocation path must help the Cleaner thread
>> too. See:
>
> Ah, I was not aware of this. So additional threads attempting to
> allocate already help out with cleaning. So moving away from the
> ReferenceHandler Thread is not such a big deal as I was thinking.
>
>> http://cr.openjdk.java.net/~plevart/jdk9-dev/removeInternalCleaner/webrev.02/src/java.base/share/classes/java/nio/Bits.java.sdiff.html
>>
>>
>> ...for the place where this is performed...
>
> I did look through your changes and I think they are good. I'd like
> to spend a little more time on the details.
>
> -Chris.
>
>> Regards, Peter
>>
>>
>>