On Fri, 2 Feb 2024 16:20:21 GMT, ExE Boss <d...@openjdk.org> wrote:
>> This patch streamlines and specializes various `String::indexOf` methods.
>> Mainly avoids the need for clamping and doing checks that are redundant in
>> almost all cases, moving the checks to the API boundary where they are
>> needed.
>>
>> This improves performance both at peak and during startup/warmup. Since
>> indexOf is heavily used in bootstrapping code it makes sense to slightly
>> dial back abstraction and delegation, which in this case also brought some
>> benefit to peak performance.
>>
>> Testing: tier1-3
>
> src/java.base/share/classes/java/lang/String.java line 2506:
>
>> 2504: fromIndex = Math.max(0, fromIndex);
>> 2505: return isLatin1() ? StringLatin1.indexOf(value, ch, fromIndex,
>> value.length)
>> 2506: : StringUTF16.indexOf(value, ch, fromIndex,
>> value.length >> 1);
>
> This needs to include the check for `fromIndex >= this.length()`:
> Suggestion:
>
> fromIndex = Math.max(0, fromIndex);
> int toIndex = length();
> if (fromIndex >= toIndex) {
> return -1;
> }
> return isLatin1()
> ? StringLatin1.indexOf(value, ch, fromIndex, toIndex)
> : StringUTF16.indexOf(value, ch, fromIndex, toIndex);
I don't think so.
If you deeply follow the invoked `indexOf()` methods, there's either a check
later, or the loop conditions are false on entry (although I'm not sure about
the intrinsic methods).
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17685#discussion_r1476293651
