RFR: 8345120: A likely bug in StringSupport::chunkedStrlenShort [v3]

[Maurizio Cimadamore]

On Fri, 29 Nov 2024 09:46:55 GMT, Per Minborg <pminborg at openjdk.org> wrote:

>> This PR proposes to rewrite the `StringSupport::chunkedStrlen*`  methods, fixing a bug in the `short_strlen` variant for odd offsets (`offset % 2 != 0`).
>> 
>> This PR also improves performance on modern hardware, as there is no need for pre-looping alignment. Removing this improves performance by about 30% for larger strings.
>> 
>> It passes the `jdk_foreign` test suit. Also, `tier1` through `tier3` passes on various platforms and configurations.
>> 
>> Base:
>> 
>> 
>> Benchmark                               (size)  Mode  Cnt    Score   Error  Units
>> InternalStrLen.changedElementQuad            1  avgt   30    2.057 ? 0.012  ns/op
>> InternalStrLen.changedElementQuad            4  avgt   30    3.776 ? 0.031  ns/op
>> InternalStrLen.changedElementQuad           16  avgt   30    6.690 ? 0.060  ns/op
>> InternalStrLen.changedElementQuad          251  avgt   30   48.581 ? 0.764  ns/op
>> InternalStrLen.changedElementQuad         1024  avgt   30  196.188 ? 3.484  ns/op
>> InternalStrLen.chunkedDouble                 1  avgt   30    1.903 ? 0.013  ns/op
>> InternalStrLen.chunkedDouble                 4  avgt   30    3.446 ? 0.025  ns/op
>> InternalStrLen.chunkedDouble                16  avgt   30    5.759 ? 0.062  ns/op
>> InternalStrLen.chunkedDouble               251  avgt   30   26.892 ? 0.141  ns/op
>> InternalStrLen.chunkedDouble              1024  avgt   30   72.940 ? 1.562  ns/op
>> InternalStrLen.chunkedSingle                 1  avgt   30    1.897 ? 0.015  ns/op
>> InternalStrLen.chunkedSingle                 4  avgt   30    5.357 ? 0.560  ns/op
>> InternalStrLen.chunkedSingle                16  avgt   30    3.821 ? 0.052  ns/op
>> InternalStrLen.chunkedSingle               251  avgt   30   19.482 ? 0.190  ns/op
>> InternalStrLen.chunkedSingle              1024  avgt   30   38.938 ? 0.411  ns/op
>> InternalStrLen.chunkedSingleMisaligned       1  avgt   30    2.230 ? 0.147  ns/op
>> InternalStrLen.chunkedSingleMisaligned       4  avgt   30    5.424 ? 0.688  ns/op
>> InternalStrLen.chunkedSingleMisaligned      16  avgt   30    9.573 ? 0.063  ns/op
>> InternalStrLen.chunkedSingleMisaligned     251  avgt   30   22.242 ? 0.182  ns/op
>> InternalStrLen.chunkedSingleMisaligned    1024  avgt   30   45.442 ? 0.252  ns/op
>> InternalStrLen.elementByteMisaligned         1  avgt   30    1.616 ? 0.041  ns/op
>> InternalStrLen.elementByteMisaligned         4  avgt   30    2.982 ? 0.018  ns/op
>> InternalStrLen.elementByteMisaligned        16  avgt   30    8.662 ? 0.085  ns/op
>> InternalStrLe...
>
> Per Minborg has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix imports

test/jdk/java/foreign/TestStringEncoding.java line 2:

> 1: /*
> 2:  * Copyright (c) 2021, 2024, Oracle and/or its affiliates. All rights reserved.

Do we have tests to check that we don't read too much? E.g. something that would trigger the problem that this PR is trying to fix?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22451#discussion_r1863475177