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

[Emanuel Peter]

On Thu, 20 Jun 2024 10:09:46 GMT, Claes Redestad <redestad at openjdk.org> wrote:

>> @wenshao I'm glad we figured it out a bit, and I hope you learned a thing or two :)
>> 
>>> It would be even better if Unsafe.putChar could be used for MergeStore in the future.
>> 
>> The `_putCharStringU` intrinsic uses `LibraryCallKit::inline_string_char_access`. It seems to generate IR nodes, I speculate it could be `StoreC`. We'd have to do more digging to see why that pattern is not accepted by `MergeStore`.
>> 
>> @wenshao I'm not sure if everything is right with the bounds checking, but I leave this to the library folks to review. What you will definately need is benchmarking not just on `M1`, but also `x86-64` and other `aarch64` architectures.
>
> I'm not opposed to accepting this patch as-is, but I think we should do so with an eye towards reverting if we figure out a way to improve the `putChar` intrinsic so that it doesn't block merge store optimization. What do you think @eme64?
> 
> As the need for the changes in [b5ad8e7](https://github.com/openjdk/jdk/pull/19626/commits/b5ad8e70928c547d134d0e4a532441cad9a7e4a2) showed added code complexity (like here) can be detrimental to performance, and if the `putChar` can be improved we might see benefits in more places.

@cl4es  @wenshao I think we should probably work on `putChar`, or at least figure out what blocks `MergeStore` for `putChar`. Can someone produce a simple stand-alone `.java` file that uses `putChar`, so that I can investigate why `MergeStore` does not work for it?

Otherwise, I agree with @cl4es : the code here may be ok for now, but we would have to revert it again later, should `MergeStore` eventually do the trick.

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

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