On Tue, 14 Apr 2026 11:13:42 GMT, Marius Hanl <[email protected]> wrote:
>> Michael Strauß has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> review comment
>
> modules/javafx.graphics/src/main/java/com/sun/scenario/effect/impl/Renderer.java
> line 310:
>
>> 308: synchronized (peerCache) {
>> 309: peers = peerCache.values().toArray(EffectPeer[]::new);
>> 310: peerCache.clear();
>
> So the cache was not cleared before, right? Is there any side effect /
> negative effect we need to be aware of?
>
> Also, can't we just iteratoe over `peerCache.values()` and call dispose on
> every peer, and then clear the `Map`? So we can save the temp `Array`.
The `EffectPeer` instances were disposed, but not removed from the map. I think
synchronization is probably completely unnecessary here, because we only ever
have a single renderer thread. I've still retained the synchronization in a
safe way to ensure no regressions. The purpose of this enhancement is not to
reevaluate whether we need synchronization at all, it's just to reduce object
allocations on the rendering path.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/2091#discussion_r3252464679
