RFR: 8333893: Optimization for StringBuilder append boolean & null [v20]

[Emanuel Peter]

On Mon, 21 Oct 2024 01:29:16 GMT, Shaojin Wen <swen at openjdk.org> wrote:

>> Shaojin Wen has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 26 additional commits since the last revision:
>> 
>>  - Merge remote-tracking branch 'origin/optim_str_builder_append_202406' into optim_str_builder_append_202406
>>  - fix build error
>>  - Merge remote-tracking branch 'upstream/master' into optim_str_builder_append_202406
>>  - Merge remote-tracking branch 'upstream/master' into optim_str_builder_append_202406
>>  - Merge remote-tracking branch 'origin/optim_str_builder_append_202406' into optim_str_builder_append_202406
>>  - Merge remote-tracking branch 'upstream/master' into optim_str_builder_append_202406
>>  - Merge remote-tracking branch 'upstream/master' into optim_str_builder_append_202406
>>  - revert test
>>  - Merge remote-tracking branch 'upstream/master' into optim_str_builder_append_202406
>>  - Merge remote-tracking branch 'upstream/master' into optim_str_builder_append_202406
>>  - ... and 16 more: https://git.openjdk.org/jdk/compare/25212971...457735c9
>
> There is no array out-of-bounds check when using Unsafe. In the appendNull method, there is already a call to ensureCapacityInternal. It is also safe to use Unsafe in this scenario.

@wenshao @cl4es I've investigated a little with the benchmark.

RangeCheck smearing could be the issue here.

![image](https://github.com/user-attachments/assets/776e8464-3e6d-4725-a6ee-3bcd772ee4e0)

It turns out that RangeChecks are smeared (a kind of elimination in straight-line code) in the same phase as the MergeStores happens (`post_loop_opts_phase`). So it seems to depend on the order of processing in the worklist, if we first remove the RC or try to merge the stores. If a RC is somewhere still stuck between stores, then merging does not quite work as hoped.

Of course if you do it all with `Unsafe`, then those RC are not there any more.

Maybe I can somehow delay the MergeStores a little more. But I have nice solution in mind - only hacks so far 😅 .

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

PR Comment: https://git.openjdk.org/jdk/pull/19626#issuecomment-2425810572