RFR: 8308588: Unnecessary synchronized on GTKStyle#ICONS_MAP can be removed

Alexey Ivanov aivanov at openjdk.org
Thu Aug 29 12:49:22 UTC 2024


On Wed, 28 Aug 2024 08:54:39 GMT, Abhishek Kumar <abhiscxk at openjdk.org> wrote:

> `static final Map<String,GTKStockIcon> ICONS_MAP` is modified only in `static` block. Then [com.sun.java.swing.plaf.gtk.GTKStyle#get](https://github.com/kumarabhi006/jdk/blob/73f7a5f15dbba54a98f3916ff1190520ac07874d/src/java.desktop/share/classes/com/sun/java/swing/plaf/gtk/GTKStyle.java#L892) method read from it within `synchronized (ICONS_MAP)` block. As this map is read-only and all its content is initialized in static block we can safely remove synchronized from get call.

It looks good to me as it is.

However, I'd go further and use either `Map.of` or `Collection.unmodifiableMap` to emphasise the fact that the map is read-only.

src/java.desktop/share/classes/com/sun/java/swing/plaf/gtk/GTKStyle.java line 893:

> 891:         GTKStockIcon stockIcon = ICONS_MAP.get(key);
> 892: 
> 893:         if (stockIcon != null) {

Suggestion:

        GTKStockIcon stockIcon = ICONS_MAP.get(key);
        if (stockIcon != null) {

Maybe remove the blank line? It used to separate a block where `stockIcon` was initialised and the condition. Now it's better to keep these two lines closer.

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

PR Review: https://git.openjdk.org/jdk/pull/20741#pullrequestreview-2268627382
PR Review Comment: https://git.openjdk.org/jdk/pull/20741#discussion_r1736137111


More information about the client-libs-dev mailing list