On Sun, 1 Mar 2026 13:36:32 GMT, Nir Lisker <[email protected]> wrote:

>> Michael Strauß has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   add test
>
> modules/javafx.graphics/src/main/java/com/sun/scenario/effect/impl/Renderer.java
>  line 116:
> 
>> 114:     }
>> 115: 
>> 116:     private final Map<PeerCacheKey, EffectPeer> peerCache = 
>> Collections.synchronizedMap(new HashMap<>(5));
> 
> Isn't a `ConcurrentHashMap` a better alternative?

No, I don't think so, and I think `Collections.synchronizedMap` is also 
pointless here. If this is supposed to allow concurrent access to the 
`peerCache` map, the original implementation is defective: 
`Collections.synchronizedMap` clearly states that all bulk access must be 
synchronized on the map instance, which this code fails to do.

I've changed the implementation to use a normal `HashMap`, and synchronize 
manually where it is accessed or modified. For this purpose, I've removed the 
`Renderer.getPeers()` method, which is only called in `PPSRenderer.dispose()` 
to iterate over the peers and dispose each one of them. Instead, the operation 
is now encapsulated in `Renderer.clearPeers()`.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/2091#discussion_r2869405013

Reply via email to