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

Alexey Ivanov aivanov at openjdk.org
Thu Aug 29 12:37:18 UTC 2024


On Thu, 29 Aug 2024 04:09:18 GMT, Abhishek Kumar <abhiscxk at openjdk.org> wrote:

> > What's the benefit of removing synchronized from the get call?
> 
> As I mentioned earlier, `ICONS_MAP` are initialized only in **static block** and after that it is just referenced to access the value out of it. Even though `ICONS_MAP` are accessed simultaneously for read operation by multiple threads, it won't lead to any inconsistent state. So, the use of Synchronized look unnecessary to me and the removal of Synchronized shouldn't affect the object state in multi threaded environment.

I agree with @turbanoff's evaluation in [the bug description](https://bugs.openjdk.org/browse/JDK-8308588#descriptionmodule): <q cite="https://bugs.openjdk.org/browse/JDK-8308588#descriptionmodule">As this map is read-only and _all its content is initialized in `clinit`_, we can safely remove `synchronized` from `get` call</q>.

We may go further and make `ICONS_MAP` unmodifiable map:


        Map<String,GTKStockIcon> iconsMap = new HashMap<>();
        iconsMap.put("FileChooser.cancelIcon", new GTKStockIcon("gtk-cancel", 4));
        iconsMap.put("FileChooser.okIcon",     new GTKStockIcon("gtk-ok",     4));
        iconsMap.put("OptionPane.yesIcon", new GTKStockIcon("gtk-yes", 4));
        iconsMap.put("OptionPane.noIcon", new GTKStockIcon("gtk-no", 4));
        iconsMap.put("OptionPane.cancelIcon", new GTKStockIcon("gtk-cancel", 4));
        iconsMap.put("OptionPane.okIcon", new GTKStockIcon("gtk-ok", 4));
        ICONS_MAP = Collections.unmodifiableMap(iconsMap);


This guarantees that `ICONS_MAP` cannot be modified; additionally the map is fully initialised by the time it's assigned to `ICONS_MAP` member.

Alternatively, as IDEA suggests, `Map.of` can be used instead:


        ICONS_MAP = Map.of(
                "FileChooser.cancelIcon", new GTKStockIcon("gtk-cancel", 4),
                "FileChooser.okIcon", new GTKStockIcon("gtk-ok", 4),
                "OptionPane.yesIcon", new GTKStockIcon("gtk-yes", 4),
                "OptionPane.noIcon", new GTKStockIcon("gtk-no", 4),
                "OptionPane.cancelIcon", new GTKStockIcon("gtk-cancel", 4),
                "OptionPane.okIcon", new GTKStockIcon("gtk-ok", 4));

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

PR Comment: https://git.openjdk.org/jdk/pull/20741#issuecomment-2317527619


More information about the client-libs-dev mailing list