On Tue, 3 Sep 2024 16:27:58 GMT, Shaojin Wen <s...@openjdk.org> wrote:
>> Use fast path for ascii characters 1 to 127 to improve the performance of
>> writing Utf8Entry to BufferWriter.
>
> Shaojin Wen has updated the pull request incrementally with one additional
> commit since the last revision:
>
> optimize for utf16
I think this looks good now. A few minor nits you can deal with as you like.
src/java.base/share/classes/java/lang/StringCoding.java line 43:
> 41: public static int countNonZeroAscii(String s) {
> 42: byte[] value = s.value();
> 43: int strlen = value.length;
Unused local variable, and `strlen` is a poor name for `value.length` (as it's
only equal to `String::length` for latin1 strings)
src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java line 322:
> 320:
> 321: /**
> 322: * Count the number of leading non-zero ascii chars in the range.
Suggestion:
* Count the number of leading non-zero ascii chars in the String.
src/java.base/share/classes/jdk/internal/classfile/impl/BufWriterImpl.java line
165:
> 163: int countNonZeroAscii = JLA.countNonZeroAscii(str);
> 164: int utflen = strlen;
> 165: if (countNonZeroAscii != strlen) {
Perhaps skip the count if `strlen` is already too large (we'll throw in the
next block)
Suggestion:
if (strlen <= 65535 && countNonZeroAscii != strlen) {
test/jdk/java/lang/String/CountNonZeroAscii.java line 49:
> 47:
> 48: for (int i = 0; i < bytes.length; i++) {
> 49: Arrays.fill(bytes, (byte) 'A');
Not sure it matters but this can be avoided if you add a `bytes[i] = (byte)
'A';` after creating `s` in the loop below.
-------------
Marked as reviewed by redestad (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/20772#pullrequestreview-2279730112
PR Review Comment: https://git.openjdk.org/jdk/pull/20772#discussion_r1743523945
PR Review Comment: https://git.openjdk.org/jdk/pull/20772#discussion_r1743530502
PR Review Comment: https://git.openjdk.org/jdk/pull/20772#discussion_r1743542942
PR Review Comment: https://git.openjdk.org/jdk/pull/20772#discussion_r1743550976
