Hey Peter,

First of all, I really love this Cleanup API!
Anyhow I think it would make sense to have Cleanable extend AutoCloseable for 
exactly the use case you rendered in your JavaDoc of the Cleaner class.

Made up example:
public <T> T execute(BufferAction bufferAction) {
  Buffer buffer = allocateBuffer();
  try (Cleanable wrapper = cleaner.register(buffer, Buffer::deallocate) {
    bufferAction.execute(buffer);
  }
}

I agree that you probably want to encapsulate the cleaner registration and 
cleaning action but for frameworks it might be beneficial to have a common 
utility method implementing a pattern such as the one above.

Chris

> On 16 May 2016, at 00:08, Peter Levart <peter.lev...@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/
> 
> 
> 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
> 

Reply via email to