RFR: JDK-8149925 We don't need jdk.internal.ref.Cleaner any more

[Roger Riggs]

Hi Peter,

I overlooked the introduction of another nested class (Task) to handle 
the cleanup.
But there are too many changes to see which ones solve a single problem.

Sorry to make more work, but I think we need to go back to the minimum 
necessary
change to make progress on this. Omit all of the little cleanups until 
the end
or do them first and separately.

Thanks, Roger




On 4/1/16 5:51 PM, Roger Riggs wrote:
> Hi Peter,
>
> Thanks for the diffs to look at.
>
> Two observations on the changes.
>
> - The Cleaner instance was intentionally and necessarily different 
> than the CleanerImpl to enable
> the CleanerImpl and its thread to terminate if the Cleaner is not 
> longer referenced.
> Folding them into a single object breaks that.
>
> Perhaps it is not too bad for ExtendedCleaner to subclass CleanerImpl 
> with the cleanup helper/supplier behavior
> and expose itself to Bits. There will be fewer moving parts. There is 
> no need for two factory methods for
> ExtendedCleaner unless you are going to use  a separate ThreadFactory.
>
> - The Deallocator (and now Allocator) nested classes are identical, 
> and there is a separate copy for each
> type derived from the Direct-X-template.  But it may not be worth 
> fixing until the rest of it is settled to avoid
> more moving parts.
>
> I don't have an opinion on the code changes in Reference, that's 
> different kettle of fish.
>
> More next week.
>
> Have a good weekend, Roger
>
>
> On 4/1/2016 12:46 PM, Peter Levart wrote:
>>
>>
>> On 04/01/2016 06:08 PM, Peter Levart wrote:
>>>
>>>
>>> On 04/01/2016 05:18 PM, Peter Levart wrote:
>>>> @Roger:
>>>>
>>>> ...
>>>>
>>>> About entanglement between nio Bits and 
>>>> ExtendedCleaner.retryWhileHelpingClean(). It is the same level of 
>>>> entanglement as between the DirectByteBuffer constructor and 
>>>> Cleaner.register(). In both occasions an action is provided to the 
>>>> Cleaner. Cleaner.register() takes a cleanup action and 
>>>> ExtendedCleaner.retryWhileHelpingClean() takes a retriable 
>>>> "allocating" or "reservation" action. "allocation" or "reservation" 
>>>> is the opposite of cleanup. Both methods are encapsulated in the 
>>>> same object because those two functions must be coordinated. So I 
>>>> think that collocating them together makes sense. What do you think?
>>>
>>> ...to illustrate what I mean, here's a variant that totally 
>>> untangles Bits from Cleaner and moves the whole Cleaner interaction 
>>> into the DirectByteBuffer itself:
>>>
>>> http://cr.openjdk.java.net/~plevart/jdk9-dev/removeInternalCleaner/webrev.13.part2/ 
>>>
>>>
>>> Notice the symmetry between Cleaner.retryWhileHelpingClean : 
>>> Cleaner.register and Allocator : Deallocator ?
>>>
>>>
>>> Regards, Peter
>>>
>>
>> And here's also a diff between webrev.12.part2 and webrev.13.part2:
>>
>> http://cr.openjdk.java.net/~plevart/jdk9-dev/removeInternalCleaner/webrev.diff.12to13.part2/ 
>>
>>
>> Regards, Peter
>>
>