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

[Claes Redestad]

On Thu, 11 Sep 2025 09:58:19 GMT, David Beaumont <duke at openjdk.org> wrote:

>> src/java.base/share/classes/jdk/internal/jimage/ImageReader.java line 356:
>> 
>>> 354:             // Unlike findNode(), this method makes only one lookup in the
>>> 355:             // underlying jimage, but can only reliably return resource nodes.
>>> 356:             if (moduleName.contains("/") || resourcePath.startsWith("/")) {
>> 
>> Slightly more efficient, here and in `containsResource` below:
>> Suggestion:
>> 
>>             if (moduleName.indexOf('/') >= 0 || resourcePath.indexOf('/') == 0) {
>
> I had thought of these micro optimizations, but I thought we preferred clarity unless performance was proven to be an issue. I see contains("<single-char>") in other code - should we be changing that?

I think replacing `startsWith(String)` with `indexOf(char)` is dubious, since the latter is O(n) and the former O(1) 

If you _really_ need to squeeze out everything on startup then `(x.length() > 0 && x.charAt(0) == '/')` is slightly faster in the interpreter (the difference more or less disappears after JIT compilation). I'd keep using `startsWith("/")`.

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

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