On Mon, 15 Nov 2021 19:10:17 GMT, Roman Kennke <rken...@openjdk.org> wrote:
> > If the intent of this change is to alter the lifetimes of the objects in > > question in a meaningful way, I recommend a CSR for the behavioral > > compatibility impact. > > It would be hard for application code to observe this change: before the > change, a ClassLoader and its classes could be lingering in the cache longer > than necessary, even if otherwise not reachable. With the change, they would > be reclaimed as soon as they become unreachable. This could only be observed, > if application code holds onto ClassLoader or Class instances via Weak or > PhantomReference, and even then I am not sure if that qualifies as > 'meaningful'. Hi Roman, What your patch changes is the following: ConcurrentHashMap<WeakReference<Class<?>>, SoftReference<ObjectStreamClass>> into: ConcurrentHashMap<WeakReference<Class<?>>, WeakReference<ObjectStreamClass>> While it is true that when the Class<?> object used in a weak key is not reachable any more by the app, it is not sensible to hold on to the value any longer so in that respect SoftReference<Value> is to "storng" of a weakness. But while the Class<?> object is still reachable by the app, the app expects to obtain the ObjectStreamClass (the value) from the cache at least most of the time. If you change the SoftReference<ObjectStreamClass> into WeakReference<ObjectStreamClass>, the ObjectStreamClass might get GC-ed even while in the middle of stream deserialization. ObjectStream class pre-dates java.lang.invoke (MethodHandles), so it uses its own implementation of weak caching. But since MethodHandles, there is a class called ClassValue<T> that would solve these problem with more elegance, because it ties the lifetime of the value (ObjectStreamClass) to the lifetime of the Class<?> key (Class object has a strong reference to the associated value) while the Class<?> key is only Weakly referenced. ------------- PR: https://git.openjdk.java.net/jdk/pull/6375
