Cleaner cleanup

[Roger Riggs]

Hi,

I haven't had time to look into this thoroughly, but since the 
at-most-once semantics
have been removed from the Phantom|Weak|SoftCleanable classes and they 
are not
used anywhere, they should be completely removed also, completing the 
cleanup.

The only function being used (except by the tests) is the primary 
support for Cleaner.register().

Roger

On 5/19/2016 6:35 AM, Christoph Engelbert wrote:
> Hey Peter,
>
> I just realized, there are two mistakes in the Javadoc code example 
> inside the Cleaner Javadoc:
>
> private final State; -> private final Statestate;
> private final Cleaner.Cleanable cleanable -> private final Cleaner.Cleanable cleanable;
>
> Chris
>
>> On 16 May 2016, at 00:08, Peter Levart <peter.levart at gmail.com 
>> <mailto:peter.levart at gmail.com>> wrote:
>>
>> Hi Roger and others,
>>
>> When the new Cleaner API was created the implementation of 
>> Cleanable(s) was split into the low-level abstract 
>> [Soft|Weak|Phantom]Cleanable classes to be used internally for 
>> purposes where the footprint matters and their corresponding 
>> CleanerImpl.[Soft|Weak|Phantom]CleanableRef subclasses used as 
>> implementations that take a Runnable cleanup action and are exposed 
>> via the public Cleaner API.
>>
>> When thinking of possible JDK internal use cases for the low-level 
>> API, I came to the conclusion that [Soft|Weak|Phantom]Cleanable 
>> classes are not suitable as is, because in cases where footprint 
>> matters, it is usually also the case that the number of 
>> [Soft|Weak|Phantom]Cleanable instances created is larger and that 
>> construction performance also matters. Especially multi-threaded 
>> construction. I'm thinking of the use cases of auto-cleanable 
>> concurrent data structures. In such use cases, the present features 
>> of [Soft|Weak|Phantom]Cleanable classes, namely the guaranteed 
>> just-once cleanup action invocation and keeping the Cleanable 
>> instance reachable until the cleanup action is performed, are 
>> actually not needed and just present footprint and performance 
>> (contention) overhead. They also present an overhead as they don't 
>> allow GC to automatically collect the Cleanable instances if the data 
>> structure containing them becomes unreachable and corresponding 
>> registered cleanup actions obsolete.
>>
>> The mentioned features are important for public Cleaner.Cleanable 
>> instances as they are usually used for cleanup of native resources 
>> where the performance of their creation is not so drastically 
>> important and where there is no intrinsic data structure to hold them 
>> reachable.
>>
>> I propose to move those features from the 
>> [Soft|Weak|Phantom]Cleanable classes down the hierarchy to the 
>> CleanerImpl.[Soft|Weak|Phantom]CleanableRef subclasses:
>>
>> http://cr.openjdk.java.net/~plevart/jdk9-dev/Cleaner.cleanup/webrev.01/ 
>> <http://cr.openjdk.java.net/%7Eplevart/jdk9-dev/Cleaner.cleanup/webrev.01/>
>>
>>
>> In this change I also removed the CleanerImpl.[Soft|Weak]CleanableRef 
>> subclasses as they are not needed and I believe will never be. I also 
>> renamed the CleanerImpl.PhantomCleanableRef subclass to 
>> CleanerImpl.PhantomCleanableImpl.
>>
>> Changes to the implementation are straightforward. The most work was 
>> put into the corresponding test. I did some clean-up to it and also 
>> changed it to accommodate for the new behavior of 
>> [Soft|Weak|Phantom]Cleanable classes. The changes speak for itself. 
>> One of the not-so obvious changes was to replace the 
>> CleanableCase.clearRef() action with the 
>> CleanableCase.releaseReferent() action. The old clearRef() action did 
>> not serve any purpose. Whether this method was called or not, the 
>> behavior of the corresponding Cleanable was unchanged as the 
>> Reference instance (referenced from the 'ref' field) was always of 
>> the same strength as the Cleanable itself. So clearing it could not 
>> affect the behavior of the Cleanable.
>>
>> I changed 'ref' to hold a direct reference to the referent and 
>> renamed the field to 'referent'. I changed the EV_XXX int constants 
>> to Event enum constants with helper methods used in 
>> CleanableCase.expectedCleanups() method that now returns the number 
>> of expected cleanup invocations - in the PhantomCleanableImpl case 
>> this is the number of expected cleanup action invocations while in 
>> the plain XxxCleanable subclass cases it is the number of 
>> Cleanable.clean() method invocations. I added the no-actions case to 
>> both PhantomCleanableImpl and XxxCleanable cases and extended the 
>> number and combinations of XxxCleanable cases.
>>
>> The checkCleaned() method was extended to verify that the number of 
>> cleanup invocations is *no more* and no less then the expected.
>>
>> See how WeakKey test is now simplified. This is the typical use-case 
>> for WeakCleanable I was talking about.
>>
>>
>> So, what do you think of this cleanup?
>>
>>
>> Regards, Peter
>>
>