On Wed, 19 Nov 2025 10:14:41 GMT, Liam Miller-Cushon <[email protected]> wrote:

>> This PR proposes adding a new overload to `MemorySegment::getString` that 
>> takes a known byte length of the content.
>> 
>> This was previously proposed in https://github.com/openjdk/jdk/pull/20725, 
>> but the outcome of 
>> [JDK-8333843](https://bugs.openjdk.org/browse/JDK-8333843) was to update 
>> `MemorySegment#getString` to suggest
>> 
>> 
>>     byte[] bytes = new byte[length];
>>     MemorySegment.copy(segment, JAVA_BYTE, offset, bytes, 0, length);
>>     return new String(bytes, charset);
>> 
>> 
>> However this is less efficient than what the implementation of getString 
>> does after [JDK-8362893](https://bugs.openjdk.org/browse/JDK-8362893), it 
>> now uses `JavaLangAccess::uncheckedNewStringNoRepl` to avoid the copy.
>> 
>> See also discussion in [this panama-dev@ 
>> thread](https://mail.openjdk.org/pipermail/panama-dev/2025-November/021193.html),
>>  and mcimadamore's document [Pulling the (foreign) 
>> string](https://cr.openjdk.org/~mcimadamore/panama/strings_ffm.html)
>> 
>> Benchmark results:
>> 
>> 
>> Benchmark                                 (size)  Mode  Cnt    Score   Error 
>>  Units
>> ToJavaStringTest.jni_readString                5  avgt   30   55.339 ± 0.401 
>>  ns/op
>> ToJavaStringTest.jni_readString               20  avgt   30   59.887 ± 0.295 
>>  ns/op
>> ToJavaStringTest.jni_readString              100  avgt   30   84.288 ± 0.419 
>>  ns/op
>> ToJavaStringTest.jni_readString              200  avgt   30  119.275 ± 0.496 
>>  ns/op
>> ToJavaStringTest.jni_readString              451  avgt   30  193.106 ± 1.528 
>>  ns/op
>> ToJavaStringTest.panama_copyLength             5  avgt   30    7.348 ± 0.048 
>>  ns/op
>> ToJavaStringTest.panama_copyLength            20  avgt   30    7.440 ± 0.125 
>>  ns/op
>> ToJavaStringTest.panama_copyLength           100  avgt   30   11.766 ± 0.058 
>>  ns/op
>> ToJavaStringTest.panama_copyLength           200  avgt   30   16.096 ± 0.089 
>>  ns/op
>> ToJavaStringTest.panama_copyLength           451  avgt   30   25.844 ± 0.054 
>>  ns/op
>> ToJavaStringTest.panama_readString             5  avgt   30    5.857 ± 0.046 
>>  ns/op
>> ToJavaStringTest.panama_readString            20  avgt   30    7.750 ± 0.046 
>>  ns/op
>> ToJavaStringTest.panama_readString           100  avgt   30   14.109 ± 0.187 
>>  ns/op
>> ToJavaStringTest.panama_readString           200  avgt   30   18.035 ± 0.130 
>>  ns/op
>> ToJavaStringTest.panama_readString           451  avgt   30   35.896 ± 0.227 
>>  ns/op
>> ToJavaStringTest.panama_readStringLength       5  avgt   30    4.565 ± 0.038 
>>  ns/op
>> ToJavaStringTest.panama_readStringLength      20...
>
> Liam Miller-Cushon has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Add a dstOffset parameter, stop using 
> StringCharBuffer/CharsetEncoder::encode

Thanks for the review!

> I think we should put some more care when going to char-based indices to byte 
> array indices, esp. if we will optimize the UTF16 case in the future

Thanks for catching that. I have made some initial updates and added an 
assertion. To confirm do you think it's OK to leave optimizing the UTF16 case 
as future work, as long as the current assumptions are clearly documented and 
guarded by assertions?

Also, I'm planning to spend more time on test coverage for this and going over 
the javadoc again.

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

PR Comment: https://git.openjdk.org/jdk/pull/28043#issuecomment-3556664930

Reply via email to