On Tue, 31 Dec 2024 01:04:24 GMT, Sergey Bylokhov <[email protected]> wrote:
>> Phil Race has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - 8336654
>> - 8336654
>
> src/java.desktop/macosx/classes/com/apple/laf/AquaUtils.java line 155:
>
>> 153: T instance;
>> 154:
>> 155: final T get() {
>
> It is used in a few places to cache the fonts/images/etc, so to be a
> "Recyclable" + "Singleton" it should store/return soft reference, and somehow
> handle the value types properly.
All uses of this are stored in static final fields and typically it is a
singleton or fixed size list/map
I don't see anything that will grow without bounds, and SoftReference isn't a
great way to manage
such cases anyway.
So I don't see any problem with doing away with SoftReference.
If we keep it, I think it is just more overhead.
And I don't see any way that isn't tricky and messy to do this whilst still
allowing value types.
It might be easier once Valhalla actually lands so we could check if it is an
identity type.
So if we keep the reference then a point fix of the Boolean case seems the
practical solution.
There's no great value to keeping a SoftRef to a Boolean so we can do without
it.
But it meant I had to look for any other similar cases by hand. I didn't find
any.
Doing this means no changes to the existing RecyclableSingleton class are
necessary to resolve the specific issue.
But I think we want to soon enough get rid of AppContext anyway, so I am moving
the ref usage
directly into RecyclableSingleton and keeping the deletion of the method from
AppContext.
The ImageCache doesn't need it. The cached images are managed by the cache code
itself.
The most recent commit implements the above but I don't see a problem with
pushing without that commit.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22868#discussion_r2665820002
