On Mon, 15 Nov 2021 21:35:06 GMT, Roman Kennke <rken...@openjdk.org> wrote:
>> The caches in ObjectStreamClass basically map WeakReference<Class> to >> SoftReference<ObjectStreamClass>, where the ObjectStreamClass also >> references the same Class. That means that the cache entry, and thus the >> class and its class-loader, will not get reclaimed, unless the GC determines >> that memory pressure is very high. >> >> However, this seems bogus, because that unnecessarily keeps ClassLoaders and >> all its classes alive much longer than necessary: as soon as a ClassLoader >> (and all its classes) become unreachable, there is no point in retaining the >> stuff in OSC's caches. >> >> The proposed change is to use WeakReference instead of SoftReference for the >> values in caches. >> >> Testing: >> - [x] tier1 >> - [x] tier2 >> - [x] tier3 >> - [ ] tier4 > > Roman Kennke has updated the pull request incrementally with two additional > commits since the last revision: > > - Use ForceGC instead of System.gc() > - Convert test to testng > 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 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 into WeakReference, the > ObjectStreamClass might get GC-ed even while in the middle of stream > deserialization. I don't quite understand this: If the Class object is still reachable by the app, 1. a weak reference would not get cleared and 2. the Class's ClassLoader would not get unloaded. Conversely, if it's not reachable by the app anymore, then the *key* in the cache would get cleared, and we would not find the ObjectStreamClass anyway. Except that the OSC holds onto the Class object by a SoftReference, so it would effectively prevent getting cleared (and get unloaded). > 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 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. Hmm, sounds nice. Do you think that would work in the context of OSC? ------------- PR: https://git.openjdk.java.net/jdk/pull/6375
