RFR: 8339290: Optimize ClassFile Utf8EntryImpl#writeTo [v17]

Claes Redestad redestad at openjdk.org
Wed Sep 4 11:09:24 UTC 2024


On Tue, 3 Sep 2024 16:27:58 GMT, Shaojin Wen <swen at 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


More information about the core-libs-dev mailing list