RFR: 8327156: Avoid copying in StringTable::intern(oop, TRAPS)

[David Holmes]

On Wed, 16 Oct 2024 07:12:20 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> The `len` parameter is just the length of the array. We use it to iterate through the array. e.g.
>> 
>>     for (int i = 0; i < len; i++) {
>>       if (value->char_at(i) != chars[i]) {
>>         return false;
>>       }
>>     }
>> 
>> But you are right that this is also the number of "code points" as defined by the Unicode standard - my apologies for that. I was mistakenly thinking that codepoints refer to actual characters, but it is a general term used for any numeric value in the unicode codespace, including non-characters and importantly surrogates. But while technically accurate I don't think using `num_unicode_points` adds any value here - quite the opposite as it obscures that this is simply the length of the array.
>
>>The len parameter is just the length of the array. We use it to iterate through the array. e.g.
> 
> Oops.
> 
>>But while technically accurate I don't think using num_unicode_points adds any value here - quite the opposite as it obscures that this is simply the length of the array.
> 
> Alright, I think it brings consistency with the fact that in the other methods we are actually passing in the number of unicode code points (like for the utf-8 case). In this case it happens to *also* be the length of the array. I think a comment in the code would help out a lot, regardless of the name chosen for the parameter.

I think we need to be very clear when we are passing in the expected length of an array as opposed to the number of characters/elements/points we intend to try to read from it. To that end just as we often use `void foo(char* buf, int buflen)` we should use e.g. `(char* utf8_str, int utf8_str_len)` if that is what we actually mean. (Though note we have an issue with using int instead of size_t for length!).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21325#discussion_r1803821741