Hi Mandy,
A couple of followups below...
On 11/3/2015 4:18 PM, Mandy Chung wrote:
On Nov 3, 2015, at 12:32 PM, Roger Riggs <roger.ri...@oracle.com> wrote:
Hi Mandy,
On 11/2/2015 2:53 PM, Mandy Chung wrote:
On Oct 20, 2015, at 11:28 AM, Roger Riggs <roger.ri...@oracle.com>
wrote:
Updated Javadoc:
http://cr.openjdk.java.net/~rriggs/cleaner-doc/
I’m happy with this API to provide an easy way to migrate away from finalizers.
Some thoughts:
1. For an existing finalizer (say cleanup function), it will be replaced with
Cleaner.create().phantomCleanable(this, this::cleanup);
Not quite, It can't reference this; the cleanup method must not have any
reference to the object
being cleaned up because the Cleanable holds a strong reference to the cleanup
state and would keep
it from being xxx unreachable.
The cleaning function must only refer to the state (and behavior) that does the
cleaning.
It could be a static method with all the state in the arguments.
Or a instance method on a object holding only the cleanup state.
Right. This was just used as an example (I didn’t pay attention to its
correctness) to lead to the comments below.
It may be useful to provide a sharable Cleaner instance that doesn’t go away.
yes, for the use cases in java.base, there will be a common cleaner in the
jdk.internal.misc package.
Should the API provide a sharable Cleaner instance for application/library to
use if they want to avoid one cleaner for each library?
It seemed like it might be an attractive nuisance because there could be
interference between
the Cleaning functions using a single Thread context. Some might be
tolerant of long delays
(For example, closing a network connections vs. others that might want
to clear some state.
For use within java.base, it would not be robust to use a Cleaner thread
shared with applications.
A shared cleaner could be added later if there there is a use case for it.
The primary use for this Cleaner API is to replace finalizers. I wonder it
worths having a convenient method easier to relate to finalizer (say
Cleaner.finalizable?)
Not just finalizers; I would stay away from the term finalizer to avoid
overloading the understanding.
Finalization is specific specification defined semantics.
2. Do you know any existing use of soft / weak references in the JDK can
benefit from this automatic cleanup mechanism? The typical cache implementation
using soft/weak references is to purge the entries when the cache is accessed
and manages the reference queue without a background thread.
Peter Levart offered an example of a self cleaning Weak key'ed collection.
It would be nice to have a ConcurrentHashMap with Weak keys.
Agree. When there is ConcurrentHashMap with Weak keys, would this
weakCleanable be used for other use cases (that’s the part unclear to me).
I’m in two minds that it sounds sensible for this Cleaner API to extend for
soft/weak references whereas I am not certain of the use cases. Anyone can
share the known use cases that would be helpful.
Besides the cases already mentioned on the thread... Anyone?
A developer at JavaOne wanted more control over the policy for clearing
SoftReferences
possibly taking into account the cost of creating the cache entry.
A technique I proposed was to use a SoftReference, not for the cached
resource itself
but as a sentinel for the VM needing to free 'soft' memory.
One possible use for a SoftCleaner is to implement a sensible cleaning
strategy for
a Cache. The CleaningFunction was would be triggered when freeing
SoftReferences
was needed by VM GC policy and could be used as a hook to use its own
algorithm
to free some of its cached values.
Updated Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-cleaner-8138696/
57 final PhantomCleanable<?> phantomCleanableList;
59 final WeakCleanable<?> weakCleanableList;
61 final SoftCleanable<?> softCleanableList;
- is there any benefit of keeping separate list for each reference type? Each
cleaner has one single ReferenceRueue and it seems that keeping one single
Cleanable list could achieve similar performance in most cases while the
implementation might be simplified.
Since each type has to extend the corresponding Reference subtype; the code was
easier to understand and maintain with separate queues. There were several
iterations using different approaches suggested during the earlier reviews.
OK. I didn’t review all iterations.
The remaining open question is whether the Cleaner API should support
weakCleanable and softCleanable or keep it for phantom cleanable only.
Mark will likely have input to this.
See above.
Thanks, Roger
111 thread.setPriority(Thread.MAX_PRIORITY);
VM threads are near max priority. The reference handler thread is MAX_PRORITY
whereas the finalizer thread is MAX_PRIORITY-2.
I'll change it to be equal to the finalizer priority; MAX_PRIORITY - 2 for the
default ThreadFactory
I think the cleaner thread should not be of MAX_PRIORITY and also perhaps the
caller may need a way to get the thread associated with a Cleaner so that users
can set the priority if needed?
The priority should be set by the ThreadFactory; if an application needs to
control the thread
it can/should provide a custom ThreadFactory; that will keep the API clean.
The setting of the default thread priority should be in the default thread
factory.
Did I look at an older webrev? The version I looked at - CleanerImpl changes
the thread priority after ThreadFactory creates it:
Thread thread = threadFactory.newThread(this);
thread.setPriority(Thread.MAX_PRIORITY);
Anyway the ThreadFactory setting the priority is good and reflected in your new
webrev:
http://cr.openjdk.java.net/~rriggs/webrev-cleaner-8138696/
Mandy
