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