On Fri, 30 Aug 2024 19:39:32 GMT, Shaojin Wen <[email protected]> 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 Utf8EntryImpl#writeTo(UTF)
So this small win now comes from reserving space up front (possibly excessively
so) and writing directly to the byte-array, avoiding excessive `reserveSpace`
calls - right? A bit underwhelming, maybe.
I also think there's a pre-existing bug here: the max length _in bytes_ of a
UTF-8 classfile entry is 65535, but we're checking `s.length() < 65535` (which
is only correct if the string is ascii). Perhaps this code need to do what
`DataOutputStream.writeUTF(String)` does and calculate the length in bytes up
front.
Did you profile the microbenchmark to see that the `Utf8EntryImpl#writeTo` is
actually a bottleneck in this test? I took a peek and it seems to be doing
mostly other things, and when I've run this test a few times it seems there's
quite a bit of run-to-run variance. I think we need a more focused
microbenchmark to say anything about the writeUTF method.
A quick, easy and portable profiling technique that doesn't depend on any
external tools: `make test TEST="micro:Utf8EntryWriteTo" MICRO="OPTIONS=-p
charType=ascii -f 1 -i 50 -prof jfr"` (bumping -i to get enough samples, -f 1
since the test runner would overwrite recordings from multiple forks).
The run will just take a recording and print something like:
JFR profiler results:
<path>/org.openjdk.bench.java.lang.classfile.Utf8EntryWriteTo.writeTo-AverageTime-charType-ascii/profile.jfr
Then use JMC or the built-in jfr tool to inspect the recording. E.g.
`build/<arch>/images/jdk/bin/jfr view hot-methods
<path>/org.openjdk.bench.java.lang.classfile.Utf8EntryWriteTo.writeTo-AverageTime-charType-ascii//profile.jfr`
is quite useful:
Java Methods that Executes the Most
Method
Samples Percent
-------------------------------------------------------------------------------------------------------
------- -------
jdk.internal.classfile.impl.SplitConstantPool.entryByIndex(int)
280 8,03%
jdk.internal.classfile.impl.RawBytecodeHelper.rawNext()
263 7,54%
jdk.internal.util.ArraysSupport.unsignedHashCode(int, byte[], int, int)
215 6,17%
jdk.internal.classfile.impl.EntryMap.firstToken(int)
210 6,02%
jdk.internal.classfile.impl.StackMapGenerator.detectFrameOffsets()
165 4,73%
java.lang.String.equals(Object)
165 4,73%
jdk.internal.classfile.impl.SplitConstantPool.findEntry(int, AbstractPoolEntry,
AbstractPoolEntry) 109 3,13%
jdk.internal.classfile.impl.SplitConstantPool.findEntry(int, AbstractPoolEntry)
104 2,98%
jdk.internal.classfile.impl.StackMapGenerator.processMethod()
85 2,44%
java.lang.classfile.constantpool.ConstantPoolBuilder.nameAndTypeEntry(String,
MethodTypeDesc) 83 2,38%
java.lang.String.<init>(byte[], byte)
76 2,18%
jdk.internal.classfile.impl.SplitConstantPool.map()
72 2,07%
jdk.internal.classfile.impl.BufWriterImpl.reserveSpace(int)
69 1,98%
jdk.internal.classfile.impl.AbstractPoolEntry$Utf8EntryImpl.toString()
64 1,84%
jdk.internal.classfile.impl.SplitConstantPool.tryFindUtf8(int, String)
63 1,81%
jdk.internal.classfile.impl.StackMapGenerator.processInvokeInstructions(...)
57 1,64%
java.nio.Buffer.checkIndex(int)
54 1,55%
java.lang.classfile.constantpool.ConstantPoolBuilder.classEntry(ClassDesc)
53 1,52%
jdk.internal.classfile.impl.SplitConstantPool$2.fetchElement(int)
52 1,49%
jdk.internal.classfile.impl.AbstractPoolEntry$Utf8EntryImpl.hashCode()
49 1,41%
java.lang.classfile.CodeBuilder.invoke(Opcode, ClassDesc, String,
MethodTypeDesc, boolean) 48 1,38%
jdk.internal.classfile.impl.AbstractPoolEntry.maybeClone(ConstantPoolBuilder,
PoolEntry) 46 1,32%
jdk.internal.classfile.impl.SplitConstantPool.maybeCloneUtf8Entry(Utf8Entry)
45 1,29%
jdk.internal.classfile.impl.StackMapGenerator.processBlock(RawBytecodeHelper)
40 1,15%
java.util.Arrays.copyOfRange(byte[], int, int)
40 1,15%```
-------------
PR Comment: https://git.openjdk.org/jdk/pull/20772#issuecomment-2322975213
PR Comment: https://git.openjdk.org/jdk/pull/20772#issuecomment-2323337370
