Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v7]
On Thu, 22 Jun 2023 05:48:34 GMT, Alan Bateman wrote: >> 温绍锦 has updated the pull request incrementally with one additional commit >> since the last revision: >> >> move HEX256 to LongCache > > src/java.base/share/classes/java/lang/Long.java line 1233: > >> 1231: HEX256[i] = (char) (((hi < 10 ? '0' + hi : 'a' + hi - >> 10) << 8) >> 1232: + (lo < 10 ? '0' + lo : 'a' + lo - 10)); >> 1233: } > > Did you mean to put this in LongCache and is the intention it be archived or > are you putting this into its own holder class? Right now it's confusing as > HEX256 is not read from the archive. i have already used a separate holder class "DigitCache", and modifiers do not use private because I will submit a new optimization using HEX256 for Integer.toHexString and Long.toHexString - PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1238135779
Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v7]
On Thu, 22 Jun 2023 02:03:55 GMT, Chen Liang wrote: > About @Glavo's VH suggestion: I think it is feasible, since the VH field is > not initialized until the method is used, so there should be no startup issue. @liach This sounds good. I'm thinking about refactoring `ByteArray` and `ByteArrayLittleEndian` to use `Unsafe` internally instead of `VarHandle` so we can use it in more places without worrying about affecting startup time. I'm running some tests to determine the benefit of doing this. - PR Comment: https://git.openjdk.org/jdk/pull/14578#issuecomment-1602118074
Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v7]
On Thu, 22 Jun 2023 00:13:06 GMT, 温绍锦 wrote: >> By optimizing the implementation of java.lang.Long#fastUUID, the performance >> of the java.util.UUID#toString method can be significantly improved. >> >> The following are the test results of JMH: >> >> Benchmark Mode Cnt Score Error Units >> UUIDUtilsBenchmark.new thrpt5 92676.550 ± 292.213 ops/ms >> UUIDUtilsBenchmark.original thrpt5 37040.165 ± 1023.532 ops/ms > > 温绍锦 has updated the pull request incrementally with one additional commit > since the last revision: > > move HEX256 to LongCache src/java.base/share/classes/java/lang/Long.java line 1233: > 1231: HEX256[i] = (char) (((hi < 10 ? '0' + hi : 'a' + hi - > 10) << 8) > 1232: + (lo < 10 ? '0' + lo : 'a' + lo - 10)); > 1233: } Did you mean to put this in LongCache and is the intention it be archived or are you putting this into its own holder class? Right now it's confusing as HEX256 is not read from the archive. - PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1238023213
Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v7]
On Thu, 22 Jun 2023 00:13:06 GMT, 温绍锦 wrote:
>> By optimizing the implementation of java.lang.Long#fastUUID, the performance
>> of the java.util.UUID#toString method can be significantly improved.
>>
>> The following are the test results of JMH:
>>
>> Benchmark Mode Cnt Score Error Units
>> UUIDUtilsBenchmark.new thrpt5 92676.550 ± 292.213 ops/ms
>> UUIDUtilsBenchmark.original thrpt5 37040.165 ± 1023.532 ops/ms
>
> 温绍锦 has updated the pull request incrementally with one additional commit
> since the last revision:
>
> move HEX256 to LongCache
I've tested the benchmarks and the patch and baseline (with extra stable
annotation) with a slightly varied version suitable for gradle run:
package com.alibaba.openjdk;
import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
import org.openjdk.jmh.annotations.Fork;
import org.openjdk.jmh.annotations.Measurement;
import org.openjdk.jmh.annotations.Mode;
import org.openjdk.jmh.annotations.OutputTimeUnit;
import org.openjdk.jmh.annotations.Warmup;
import org.openjdk.jmh.infra.Blackhole;
import java.util.UUID;
import java.util.concurrent.TimeUnit;
@BenchmarkMode(Mode.Throughput)
@OutputTimeUnit(TimeUnit.MILLISECONDS)
@Warmup(iterations = 3, time = 10)
@Measurement(iterations = 6, time = 5)
@Fork(1)
public class UUIDUtilsBenchmark {
public static UUID uuid = UUID.randomUUID();
@Benchmark
public void jdk(Blackhole bh) {
bh.consume(uuid.toString());
}
@Benchmark
public void fast(Blackhole bh) {
bh.consume(UUIDUtils.fastUUID(uuid));
}
}
The throughput varies a lot between iterations somehow; the patch and baseline
with stable has no significant difference (i.e. within the error range, about
10%)
-
PR Comment: https://git.openjdk.org/jdk/pull/14578#issuecomment-1601953110
Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v7]
On Thu, 22 Jun 2023 00:13:06 GMT, 温绍锦 wrote: >> By optimizing the implementation of java.lang.Long#fastUUID, the performance >> of the java.util.UUID#toString method can be significantly improved. >> >> The following are the test results of JMH: >> >> Benchmark Mode Cnt Score Error Units >> UUIDUtilsBenchmark.new thrpt5 92676.550 ± 292.213 ops/ms >> UUIDUtilsBenchmark.original thrpt5 37040.165 ± 1023.532 ops/ms > > 温绍锦 has updated the pull request incrementally with one additional commit > since the last revision: > > move HEX256 to LongCache Changes requested by liach (Author). About @Glavo's VH suggestion: I think it is feasible, since the VH field is not initialized until the method is used, so there should be no startup issue. On a side note, JDK itself has a `UUIDBench` that benchmarks toString as well: can run it with `make test TEST="micro:java.util.UUIDBench.toString"` once the configuration has JMH set up, which I will be using (against master and this patch) src/java.base/share/classes/java/lang/Long.java line 484: > 482: > 483: buf[off] = (byte) (i >> 8); > 484: buf[1 * charSize + off] = (byte) i; Suggestion: buf[off] = (byte) (i0 >> 8); buf[1 * charSize + off] = (byte) i0; Doesn't compile on my end - PR Review: https://git.openjdk.org/jdk/pull/14578#pullrequestreview-1492220846 PR Comment: https://git.openjdk.org/jdk/pull/14578#issuecomment-1601915404 PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1237906554
Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v7]
On Thu, 22 Jun 2023 00:13:06 GMT, 温绍锦 wrote: >> By optimizing the implementation of java.lang.Long#fastUUID, the performance >> of the java.util.UUID#toString method can be significantly improved. >> >> The following are the test results of JMH: >> >> Benchmark Mode Cnt Score Error Units >> UUIDUtilsBenchmark.new thrpt5 92676.550 ± 292.213 ops/ms >> UUIDUtilsBenchmark.original thrpt5 37040.165 ± 1023.532 ops/ms > > 温绍锦 has updated the pull request incrementally with one additional commit > since the last revision: > > move HEX256 to LongCache The Bots have removed the warning so the titles match. Please wait 24hrs to integrate to give anyone who has commented a chance to review and approve. - PR Comment: https://git.openjdk.org/jdk/pull/14578#issuecomment-1601884222
Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v7]
On Thu, 22 Jun 2023 01:03:11 GMT, Roger Riggs wrote: > fyi, the title of this PR need to match exactly the title of the bug > [JDK-8310502](https://bugs.openjdk.org/browse/JDK-8310502). The mismatch is > an Integration blocker. (See the comment in the description). i have made the changes. is it ok now? - PR Comment: https://git.openjdk.org/jdk/pull/14578#issuecomment-1601878855
Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v7]
On Thu, 22 Jun 2023 00:13:06 GMT, 温绍锦 wrote: >> By optimizing the implementation of java.lang.Long#fastUUID, the performance >> of the java.util.UUID#toString method can be significantly improved. >> >> The following are the test results of JMH: >> >> Benchmark Mode Cnt Score Error Units >> UUIDUtilsBenchmark.new thrpt5 92676.550 ± 292.213 ops/ms >> UUIDUtilsBenchmark.original thrpt5 37040.165 ± 1023.532 ops/ms > > 温绍锦 has updated the pull request incrementally with one additional commit > since the last revision: > > move HEX256 to LongCache fyi, the title of this PR need to match exactly the title of the bug [JDK-8310502](https://bugs.openjdk.org/browse/JDK-8310502). The mismatch is an Integration blocker. (See the comment in the description). - PR Comment: https://git.openjdk.org/jdk/pull/14578#issuecomment-1601874909
