On Thu, 11 Sep 2025 09:58:19 GMT, David Beaumont <[email protected]> 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
