RFR: 8367005: ImageReader refactor caused performance regressions for startup and footprint [v2]

David Beaumont duke at openjdk.org
Thu Sep 11 00:36:20 UTC 2025


On Wed, 10 Sep 2025 23:57:36 GMT, Claes Redestad <redestad at openjdk.org> wrote:

>> David Beaumont has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Found additional place where new API can be used.
>
> src/java.base/share/classes/jdk/internal/jimage/ImageReader.java line 379:
> 
>> 377:             // exist, so it skips checking the nodes cache and only checks for
>> 378:             // an ImageLocation.
>> 379:             if (moduleName.contains("/") || resourcePath.startsWith("/")) {
> 
> The fast-path guard here and above might improve if they were done before synchronizing, e.g. dropping the modifier and putting a `synchonized (this)` block around the rest of the method. 
> 
> Putting the likely cheaper `resourcePath.startsWith` first might be a small win, too, depending on input/benchmark.

When called from SystemModuleFinders this really should never happen (arguably it's IAE) so it won't make a difference per-call if it's changed, and it won't really help reduce the time of the locked section.
Both checks will basically always be done, so ordering isn't an issue (but a fair point to raise).

Module names are typically short, so while a string scan is not ideal, it's not awful (and we do string concatentaion later anyway). Ideally there would be a ModuleName type to use as the parameter.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27203#discussion_r2338229587


More information about the core-libs-dev mailing list