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

[Vladimir Kozlov]

On Mon, 14 Dec 2020 18:25:07 GMT, John R Rose <jrose at openjdk.org> wrote:

>> 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.

I agree with John that these changes are moving in wrong direction.
I would like to know why Escape Analysis does not eliminate substring object if it is only used for StartsWith():
 public static boolean foo(String s) { 
     return s.substring(1).startsWith("a"); 
 }
Improving EA will get you more benefits.

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

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