On Wed, 26 Aug 2026 13:54:55 GMT, Kevin Rushforth <[email protected]> wrote:
>>> I prototyped it using an abstract class, which I like better than an >>> interface for this case, so I might go with it. >> >> That can’t use `extends WeakReference` > > Ah, good point. And having `WeakImpl` extend `WeakReference` would eliminate > a second object for each reference wrapper (and a level of indirection on the > get call). Given that we already have follow-up issues to consider eliminating this WeakReferenceWrapper, I've decided to leave this wrapper as it currently stands: a single concrete helper class with two fields. I'll file a follow-up issue to improve it if it ends up sticking around. It's a cleaner implementation and better encapsulation using a class (which matters because the wrapper class needs to be public since it is used from more than one package), even though there is an extra indirection when calling `wr.get()`. Reworking it to eliminate the extra indirection, while keeping the existing encapsulation, would be possible with a little work, but it isn't worth it at this time. We don't expect a huge number of these and they aren't in the hot path (they are short-lived items created and used only when items are removed). I did notice a couple typos in the docs, which I'll fix, but I'll defer any other improvements until we know whether it is worth the effort. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2250#discussion_r3865186189
