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 <[email protected]> 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
>