[foreign-memaccess+abi] RFR: 8313238: Improve the performance of strlen for null-terminated strings [v2]

[Jorn Vernee]

On Mon, 7 Aug 2023 08:25:13 GMT, Per Minborg <pminborg at openjdk.org> wrote:

>> This PR proposes to use native calls for determining the string length for C-type strings. 
>> 
>> Smaller segments (<1024 bytes) is using a trivial call whereas all other segments are using a normal call. We might consider always using regular calls if we think most segments are unbound anyhow.
>> 
>> The PR also contains a number of new tests, one of which requires a large heap size to run. It is likely that this latter test will not run in most test environments. In such cases, it is silently ignored.
>
> Per Minborg has updated the pull request incrementally with three additional commits since the last revision:
> 
>  - Remove redundant check
>  - Use strnlen
>  - Improve exception handling

src/java.base/share/classes/jdk/internal/foreign/StringSupport.java line 122:

> 120: 
> 121:     private static int native_strlen_byte(MemorySegment segment, long start) {
> 122:         try {

I think this `try` block is only meant to deal with a potential exception from `invokeExact`? But, since it covers many lines of code, it's hard to tell what it's doing.

I suggest trying to limit the covered lines, for example by creating out-of-line wrapper methods for `STRNLEN` and `STRNLEN_TRIVIAL` and then putting the try/catch only in there.

src/java.base/share/classes/jdk/internal/foreign/StringSupport.java line 131:

> 129:                 if (segmentSize < MAX_TRIVIAL_SIZE) {
> 130:                     len = (int)STRNLEN_TRIVIAL.invokeExact(segment, (int) segmentSize);
> 131:                 } else if (segmentSize < Integer.MAX_VALUE) {

size_t is unsigned. So it should be fine to use the native variant as long as the upper 32 bits are not set.

src/java.base/share/classes/jdk/internal/foreign/StringSupport.java line 137:

> 135:                     // to a Java method. It is possible to use a reduction of several STRNLEN invocations
> 136:                     // in a future optimization.
> 137:                     len = strlen_byte(segment);

Can't we just always throw here? `len` is not allowed to be larger than an `int` any ways.

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

PR Review Comment: https://git.openjdk.org/panama-foreign/pull/857#discussion_r1285612785
PR Review Comment: https://git.openjdk.org/panama-foreign/pull/857#discussion_r1285602480
PR Review Comment: https://git.openjdk.org/panama-foreign/pull/857#discussion_r1285604581