RFR: 8305746: InitializeEncoding should cache Charset object instead of charset name [v2]

[Naoto Sato]

On Tue, 18 Apr 2023 11:34:45 GMT, Peter Hofer <phofer at openjdk.org> wrote:

>> Store `Charset` object in `jnuEncoding` and use `String(byte[], Charset)` and `String.getBytes(Charset)` instead of passing the charset name.
>
> Peter Hofer has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains one commit:
> 
>   8305746: InitializeEncoding should cache Charset object instead of charset name

Looks good to me (with minor nits). Since it would be hard to write a regression test, what kind of tests have you done? Did you run your patch on platforms that made sure your new code path work?

src/java.base/share/native/libjava/jni_util.c line 628:

> 626: 
> 627: static int fastEncoding = NO_ENCODING_YET;
> 628: static jobject jnuEncoding = NULL;

Maybe `jnuCharset` is clearer that it is a `Charset` object.

src/java.base/share/native/libjava/jni_util.c line 757:

> 755:                 jnuEncoding = (*env)->NewGlobalRef(env, charset.l);
> 756:                 (*env)->DeleteLocalRef(env, charset.l);
> 757:                 break;

Could return immediately

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

PR Review: https://git.openjdk.org/jdk/pull/13499#pullrequestreview-1390934727
PR Review Comment: https://git.openjdk.org/jdk/pull/13499#discussion_r1170569204
PR Review Comment: https://git.openjdk.org/jdk/pull/13499#discussion_r1170568134