Cleaner cleanup

[Claes Redestad]

Hi Peter,

I for one think this looks like a very nice cleanup.

The patch drops 5 classes from a minimal VM startup test, which is a 
welcome improvement.

/Claes

On 2016-05-16 00:08, Peter Levart 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/
>
>
> 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
>