RFR: JDK-8149925 We don't need jdk.internal.ref.Cleaner any more

[Peter Levart]

Hi Martin,

As I have been studying the CleanerTest too recently, let me comment on 
some questions...

On 05/09/2016 06:34 PM, Martin Buchholz wrote:
> ---
>
>          Assert.assertEquals(map.get(k2), data, "value should be found
> in the map");
>          key = null;
>          System.gc();
>          Assert.assertNotEquals(map.get(k2), data, "value should not be
> found in the map");
>
> I initially expected this to fail at least intermittently because a
> cleaner may not yet have removed the entry from the map.

...The cleaner need not remove the entry from the map in order for this 
code to succeed. The sufficient condition is that GC discovers the k1 
and k2 WeakReference(s) and clears them. When a WeakKey is cleared it is 
only equal to itself since the referent is gone (see equals() method). 
So it is important to ensure that the referent remains reachable at 
least until the 1st assert above (perhaps by inserting 
Reference.reachabilityFence(key) just after it)...

>
> Why aren't we calling whitebox.fullGC() everywhere where System.gc()
> is being called?
>
> What's particularly confusing here is that the WeakKey is not actually
> surely removed from the map, but instead two WeakKeys that may have
> been equal become non-equal when they get cleared by the gc.  Which is
> generally bad practice.  Which suggests we wouldn't implement a real
> public concurrent map of weak keys this way.

Why not? We don't need to lookup the value after the WeakKey(s) are 
cleared, since by definition, we can't reach their referent any more 
(the 'key'). But the instance of the WeakKey that is inserted in the map 
can still be removed because it is the same instance that is referenced 
by the WeakCleanable and a cleared WeakKey is still equal to itself. So 
we have the following invariants:

- while the key (the referent) is still reachable, multiple WeakKeys 
with the same referent are equal among themselves so a WeakKey can be 
constructed that is equal to some other WeakKey in the map.
- after the key (the referent) becomes weakly-reachable and GC kicks-in, 
all WeakReferences with referents from the selected set are cleared 
atomically, including WeakKey(s) and WeakCleanable(s) refering to the 
same key referent, so only the instance of the WeakKey that is inserted 
in the map can be used to remove the stale entry from it.

You can find a simpler implementation of WeakKey in my "Cleaner cleanup" 
proposal where WeakKey is a subclass of WeakCleanable and simply removes 
itself from the map when its clean() method is called by the Cleaner 
thread...

>
> ---
>
> WeakCleanable and friends are defined in jdk.internal, don't appear to
> be used at all, and since there are fields in CleanerImpl, they impose
> a hidden tax on all users of Cleaner.  I don't see why there is both
> WeakCleanable (using subclassing) and WeakCleanableRef (using a
> provided Runnable).  Is a compelling use case for WeakCleanables
> forthcoming?

I have a few ideas that I would like to present, but I first wanted to 
propose making [Soft|Weak]Cleanable(s) even more low-level to make those 
use cases feasible. See the "Cleaner cleanup" thread for more info on that.


Regards, Peter