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