On Tue, 18 Apr 2023 11:34:45 GMT, Peter Hofer <pho...@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
