Cleaner cleanup

[Peter Levart]

Hi Roger,

Here's the same webrev with an example of use of WeakCleanable that I 
have been thinking to propose later (I have done this in 5 minutes):

http://cr.openjdk.java.net/~plevart/jdk9-dev/Cleaner.cleanup/webrev_with_ClassLoader_exmaple/

This example uses WeakCleanable as a lightweight cleanup mechanism that 
removes entries from ClassLoader's lock map as soon as they are not 
needed any more - usually this is immediately after the class has been 
loaded. This keeps the map size from growing and has a nice impact on 
final VM footprint.

So the question is whether it is worth keeping those superclass(es) for 
such and similar cases. It's not difficult to add them back if/when 
needed. The most work is in the test.


Regards, Peter

On 05/19/2016 03:23 PM, Roger Riggs wrote:
>
> 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> 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
>>>
>>
>