On Tue, 1 Jul 2025 14:09:49 GMT, David Beaumont <[email protected]> wrote:
>> David Beaumont has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Linting of a few minor issues (nothing serious).
>>
>> * Linting of minor issues.
>> * Factored out the module existence test, it's only a performance
>> heuristic and could (should?) be removed.
>
> src/java.base/share/classes/jdk/internal/jimage/ImageReader.java line 255:
>
>> 253: if (openers.isEmpty()) {
>> 254: close();
>> 255: // TODO (review note): Should this be synchronized
>> by "this" ??
>
> I genuinely this this might be an existing issue.
It would be safer synchronized (especially `findNode`), since the nodes are
shared and another thread might be accessing nodes.
But it might drop performance even in the non-contended case.
The alternative would be to use a ConcurrentHashMap but it has its own costs.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26054#discussion_r2205768773
