On Sat, 7 Sep 2024 08:51:25 GMT, Shaojin Wen <s...@openjdk.org> wrote:
>> PR #20772 introduced an optimization for writeUTF, which can also be used in >> DataOutputStream::writeUTF. > > Shaojin Wen has updated the pull request incrementally with one additional > commit since the last revision: > > reduce JDKUTF#utflen codeSize src/java.base/share/classes/java/io/ObjectOutputStream.java line 2014: > 2012: } > 2013: > 2014: void writeUTF(String str, int stroff) throws IOException { Can we move this to be near `writeUTFInternal` and make this `private` as they are closely related? This should also be renamed to indicate this is internal, like `writeMoreUtf` to avoid misuse. src/java.base/share/classes/jdk/internal/classfile/impl/BufWriterImpl.java line 182: > 180: > 181: for (int i = countNonZeroAscii; i < strlen;) { > 182: offset = JDKUTF.putChar(elems, offset, str.charAt(i++)); Can we put this `++` in for loop instead of the statement here? src/java.base/share/classes/jdk/internal/util/JDKUTF.java line 35: > 33: * @since 24 > 34: */ > 35: public abstract class JDKUTF { Can we name this class `ModifiedUtf`? `JDKUTF` looks like a constant field name, and this format is part of Java Platform instead of just JDK. src/java.base/share/classes/jdk/internal/util/JDKUTF.java line 37: > 35: public abstract class JDKUTF { > 36: @ForceInline > 37: public static int putChar(byte[] buf, int offset, char c) { Do you think we can just move the loop here, like: public static int putChars(byte[] buf, int offset, String str, int strStart, int strEnd) { for (int i = strStart; i < strEnd; i++) { offset = putChar(buf, offset, str.charAt(i); } } We can even unroll putChar in the loop. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20886#discussion_r1759191218 PR Review Comment: https://git.openjdk.org/jdk/pull/20886#discussion_r1759175406 PR Review Comment: https://git.openjdk.org/jdk/pull/20886#discussion_r1759172395 PR Review Comment: https://git.openjdk.org/jdk/pull/20886#discussion_r1759205885