RFR: 8360037: Refactor ImageReader in preparation for Valhalla support [v7]

[Roger Riggs]

On Tue, 1 Jul 2025 14:09:49 GMT, David Beaumont <duke at openjdk.org> 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