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

[Vladimir Ivanov]

>> - You introduce a call to a method (String::startsWith(String, int))
>> which is not present in bytecode. It means (unless the method is called
>> from different places) there won't be any profiling info present, and
>> even if there is, it is unrelated to the call site being compiled. If
>> the code is recompiled at some point (e.g., it hits an uncommon trap in
>> startsWith(String,int) overload), there won't be any re-profiling
>> happening. So, it can hit the very same condition on the consequent
>> recompilations.
>>
> Thank you to raise this! I didn't consider the profiling data before.
> it is an issue. Maybe it can explain why I observe only 80% performance of hand-craft code.
> 
> TBH, I haven't had a clear and solid answer for it yet.
> The transformation actually won't alter control-flow. Perhaps, I can find a way to amend methoddata from the old method.
> Or, is that possible I request to reprofile it after I modify some bytecodes?

Sorry, I overlooked your question.

To be clear: the transformations during C2 compilation don't affect 
bytecodes in any way.

If a method is inlined, a deoptimization may lead to a surprising 
resumption of execution in the middle of unexpected method, but it's a 
one time occurrence and consequent executions will follow original path 
in the bytecode.

If inlining doesn't happen, then the JVM should be able to finish 
successully finish profiling in the callee if needed (reprofile request 
invalidates the nmethod and it forces the new call in C2 to go through 
either interpreter or C1 until it is compiled with C2).

So, I'm concerned specifically about inlined case. And I consider it a 
blocking issue for adapting the proposed approach of rewriting rules in 
C2. I don't see a way to reliably fix it without intrinsifying the 
operation.

Best regards,
Vladimir Ivanov

> 
>> - "s.startsWith(prefix)" call node is reused and rewired to call
>> unrelated method "base.startsWith(prefix, beg)". Will the new target
>> method be taken into account during inlining? I would be much more
>> comfortable seeing fresh call populated using standard process
>> (involving CallGenerators et al) which then substitutes the original
>> node. That way you make it less fragile longer term.
>>
> I can make that happen. actually, I generated a brand-new call-generator in early revision of this patch.
> I drop it because it requires more code.
> 
>> - "hopefully c2 optimizer can remove the used substring()"
>> If everything is inlined in the end, it can happen, but it's fragile.
>> Instead, you could teach C2 that the method is "pure" (no interesting
>> side effects to care about) and cut the call early. It already happens
>> for boxing methods (see LateInlineCallGenerator::_is_pure_call for
>> details).
>>
> I do have the exact logic in early revision. I remarked substring() as late-inlining candidate and pure.
> I delete it because I found substring() is "always" inlined and c2 optimizer can do the deadcode elimination job if nobody uses it.
> I understand your concern.  I can improve it in the future.
> 
>> Overall, if you want to keep the enhancement C2-specific, I'd suggest to
>> look into intrinsifying String::startsWith(String, int) and not relying
>> on its bytecodes at all. That way you would avoid fighting against the
>> rest of the JVM in some situations.
>>
> Because this optimization isn't one-off thing for startsWith, I can't intrinsify it.
> I'd like to explore an extensible approach.
> 
>> Best regards,
>> Vladimir Ivanov
> 
> -------------
> 
> PR: https://git.openjdk.java.net/jdk/pull/974
>