RFR: 8254807: Optimize startsWith() for String.substring() [v5]

[John R Rose]

On Mon, 14 Dec 2020 06:10:13 GMT, Xin Liu <xliu at openjdk.org> wrote:

>> The optimization transforms code from s=substring(base, beg, end); s.startsWith(prefix)
>> to substring(base, beg, end) | base.startsWith(prefix, beg). 
>> 
>> it reduces uses of substring. hopefully c2 optimizer can remove the used substring.
>
> Xin Liu has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fix build failure after merging

This is a lot of churn and a lot of code surface area to gain a small benefit.  I don't think this sort of change takes our code base in a healthy direction.  Is it really worth it?

Somebody deeply familiar with the existing string-builder optimizations needs to look at this carefully, and make sure that it is maintainable going forward into the future.  Once this integrates, we pay for the maintenance forever.

Even more basically, I think it may be an erroneous transformation.  Given `base = "abcd"` and `s = base.substring(base, 1, 2)` (i.e., `base = "b"`), the boolean `z1 = s.startsWith("bc")` is false even though the boolean `z2 = base.substring("bc", 1)` is true.  This means it is invalid to replace `z1` with `z2`.  If the optimization were to make this transform, deeply unpredictable things could happen in an application.

Now, I looked for the conditional that excludes this particular error case.  I didn't find it.  This tells me that one of two things is true:  (a) The proposed change is incorrect, or (b) the proposed change is so subtle it will be impossible to maintain in a state of correctness.

For these reasons, I am skeptical that (a) the change should be integrated, and (b) any similar change should be integrated.

Somebody who is deeply familiar with the existing string optimizations may have a different opinion, and I would be happy to accept their reassurances.

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

PR: https://git.openjdk.java.net/jdk/pull/974