[External] : Re: SoV-2: weak references

Wed Feb 9 18:23:35 UTC 2022

This is not based on a very deep understanding, sorry:

* WeakReference, 1-arg constructor: When given a bucket 2-or-3 instance, it
seems maybe fine to just treat as strong reference? User is saying "just
hold this loosely, don't keep it alive on my account" and doesn't care what
happens after that.

* WeakReference, 2-arg constructor: this already seems to depend on
identity, so bucket 2 or 3 is more clearly problematic now. That this type
wanted to become a value at all suggests the code might *already* be a bit
broken. Seems worth an error or warning just for trying to do this at all.
The behavior (if the warning is ignored) only has to be something "simple"
and reasonable, since what it's being asked to do is arguably nonsensical.

* For WeakHashMap use cases, I would expect that any attempts to actually
design something "good" that does what users really "want" would end up at
some different library entirely. This is something that Caffeine
<https://www.javadoc.io/doc/com.github.ben-manes.caffeine/caffeine/2.0.3/com/github/benmanes/caffeine/cache/Caffeine.html>
probably does well (disclosure, I worked on the thing that became that).

On Wed, Feb 9, 2022 at 10:02 AM Brian Goetz <brian.goetz at oracle.com> wrote:

> I have come around to a similar conclusion.  None of the models for
> WR(value) are really all that justifiable, other than "throws".  But we
> dislike that because so much code uses WHM that we are worried about this
> code all of a sudden failing.  But we are trying to fix that by distorting
> WR, rather than enhancing WHM.
>
> WHM has its own set of constraints; it is the closest thing to a cache in
> the JDK (with a very rigid eviction policy), and frequently gets used as
> such.  But implicit in the use of a cache is something that only the code
> that uses WHM knows: are we caching because we're trying to avoid redundant
> computation, or are we caching because we *cannot* recompute the mapping?
> The current eviction strategy is consistent with both -- for identities.
> But once we get past identity objects, I can very well imagine some clients
> wanting to aggressively release mappings from value keys (to save memory),
> and some wanting to never release mappings from value keys (because they
> need a unique target).  Which suggests that creating a WHM requires a
> user-specifiable policy; I can think of cases where each of the following
> policies are sensible (note that they all avoid getting into the details of
> invalidation):
>
>  - THROW -- throw when someone uses a value key.  This would probably be
> the default.
>  - KEEP -- keep value key mappings forever.  This is useful when we are
> computing, say, histograms of "how often have we seen key X".
>  - DISCARD -- clear value key mappings immediately.
>  - SOFT -- Keep value key mappings around for as long as there is not
> excessive heap pressure.
>
> For policy SOFT, we would likely implement by wrapping a SoftReference
> around an *entire* side map of key -> value mappings for value keys.
>
> Then we'd provide some new factories for WHM to select one of these
> policies.
>
> On 2/9/2022 11:50 AM, Dan Heidinga wrote:
>
> One option is to look at what we can do to help users prepare for IAE
> when using value-based classes as keys to WHM.  Can we take an
> approach similar to JEP 390 [1] and provide JFR events that flag uses
> of value-based classes as keys?  It's not perfect but similar to JEP
> 390, it does help users to know if they need to do something to
> prepare for this.
>
> --Dan
>
> [1] http://openjdk.java.net/jeps/390
>
> On Thu, Jan 20, 2022 at 1:54 PM Dan Heidinga <heidinga at redhat.com> <heidinga at redhat.com> wrote:
>
> It certainly seems that all the choices are bad.
>
> The "obvious" choice is to simply say that WeakReference<Value> makes no sense, in that it sidesteps the hard semantics questions.
>
> It's an uncomfortable answer but it seems to provide the most
> defensible (and thus understandable) semantics for users. Values don't
> have an explicit lifetime and thus there is no way to tell when "this"
> copy of a value goes out of scope and can be collected. The object
> references (if any) held by the value are not a good proxy for its
> lifecycle - they can have either shorter or much longer life spans -
> and will make reasoning about when a WeakReference<Value> can be
> collected difficult for experts, let alone most users.
>
>
> My fear is that this will cause new failures, where existing libraries that toss objects into WHMs to cache derived results, will start to experience new failures when the value types show up in the heap (after all, WeakReference::new takes Object.)
>
> This may be a case where the WeakReference constructor needs to be
> updated to take an IdentityObject and the old constructor marked as
> @Deprecated? Which doesn't solve the immediate problem but helps
> justify adding a "fail-fast" check to all WeakReference constructors
> so that they throw an IllegalArgumentException if the referent isn't
> an IdentityObject.
>
> This won't avoid failures but it does make it very clear what went
> wrong rather than introducing "strange", hard to diagnose failures.
>
>
> And we'll have to have something to tell those users, because they declared a WeakHashMap<User, UserData>, and someone created a value subtype of User -- which seems entirely valid.
>
> It is possible we could do something special with WHM, since it is layered atop WR, but that still begs the question -- what?
>
> Starting from the conclusion that WeakReference<Value> is a
> meaningless entity, what are the options here?
>
> 1) Make it impossible to use a Value as a key in a WeakHashMap.
> ::put(key, value) & ::pulAll(Map m) will throw if a key is a Value
> object.  ::containsKey(Object) will always be false if the Object is a
> ValueObject.  This makes WeakHashMap unusable with Values.  The
> semantics are clear but all existing uses of WeakHashMap will need to
> be adapted to defensively check for Values and do something (tbd) to
> avoid the exceptions.
>
> 2) Use strong references for Value keys in WeakHashMap.
> Treat each Value object used as a key in WeakHashMap as a strong
> reference.  This means Value keys will never be removed and will keep
> their corresponding map value alive forever (or until explicitly
> removed).  While this will allow WeakHashMaps to continue to be used
> as Maps for Values, it will break the contract for WHM and may
> introduce memory leaks into otherwise correct programs.
>
> 3) Pick some other object to act as the reference when using a Value
> key in a WHM.
> This is basically the solution we rejected for WeakReference<Value>
> and all the same problems apply here.  It may allow existing code to
> "keep working" when it first deals with Values but introduces strange
> failure cases and difficult to reason about rules.  It avoids
> exceptions but leaves the code doing the wrong thing with no way to
> tell.
>
> Anyone see another option here?
>
> --Dan
>
>
>

-- 
Kevin Bourrillion | Java Librarian | Google, Inc. | kevinb at google.com