Re: RFR: 8327858: Improve spliterator and forEach for single-element immutable collections [v2]
On Fri, 26 Apr 2024 22:11:07 GMT, Chen Liang wrote: >> Do we need additional tests or are these modifications already covered by >> the existing tests? > > @minborg I have added a test as part of Collection mother-of-all-tests to > ensure spliterator and forEach yields in the same order as iterator for > unmodifiable/immutable collections. One thing of note is that somehow `==` > for yields fail for some collections like > `unmodifiableSequencedMap(linkedHashSet).sequencedKeySet()` so I have to use > `Objects.equals` instead. @liach > One thing of note is that somehow `==` for yields fail for some collections > like `unmodifiableSequencedMap(linkedHashSet).sequencedKeySet()` so I have > to use `Objects.equals` instead. Actually, it’d be for `unmodifiableMap(…).entrySet()` and `Map.of(…).entrySet()`, as those create a new `UnmodifiableEntry` and `KeyValueHolder` on iteration respectively. `unmodifiableSequencedMap(…).sequencedKeySet()` should work with `==`. - PR Comment: https://git.openjdk.org/jdk/pull/15834#issuecomment-2081284896
Re: RFR: 8327791: Optimization for new BigDecimal(String) [v18]
On Sat, 27 Apr 2024 10:48:38 GMT, Shaojin Wen wrote: >> Shaojin Wen has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 22 additional >> commits since the last revision: >> >> - Merge remote-tracking branch 'upstream/master' into optim_dec_new >> - Merge remote-tracking branch 'upstream/master' into optim_dec_new >> - use while instead for >> - Update src/java.base/share/classes/java/math/BigDecimal.java >> >>Co-authored-by: Claes Redestad >> - bug fix for CharArraySequence#charAt >> - bug fix for CharArraySequence >> - fix benchmark >> - one CharArraySequence >> - restore comment >> - easier to compare >> - ... and 12 more: https://git.openjdk.org/jdk/compare/47953a00...bb620ba3 > > In money-related scenarios, such as product prices, etc., BigDecimal needs to > be used instead of float/double. I focus on optimizing the performance of > BigDecimal's construction and toString scenarios. > > Constructing BigDecimal usually includes two scenarios: > * new BigDecimal(char[], int, int), Many libraries, such as mysql > drveri/fastjson/jackson and other commonly used libraries use this function > to construct BigDecimal > * new BigDecimal(String), There are also many scenarios, such as data > integration scenarios, scenarios where json is read from the message queue > and then converted into target database row records, often using new > BigDecimal(String) > > I did a performance comparison test on Mac Book M1 Pro. The compared branches > are: > * [baseline](https://github.com/wenshao/jdk/tree/upstream_master_0312) > https://github.com/wenshao/jdk/tree/upstream_master_0312 > * bb620ba [Full](https://webrevs.openjdk.org/?repo=jdk=18177=17) - > [Incremental](https://webrevs.openjdk.org/?repo=jdk=18177=16-17) > ([bb620ba3](https://git.openjdk.org/jdk/pull/18177/files/bb620ba39a6f1ce17ff273bac54ebb709beb1667)) > > > # 1. new BigDecimal(String) benchmark > Execute the following commands respectively > > make test TEST="micro:java.math.BigDecimals.testConstructorWithHugeString" > make test TEST="micro:java.math.BigDecimals.testConstructorWithLargeString" > make test TEST="micro:java.math.BigDecimals.testConstructorWithSmallString" > make test TEST="micro:java.math.BigDecimals.testConstructorWithString" > > > ## benchmark result > > -Benchmark Mode CntScore Error > Units #baseline > -BigDecimals.testConstructorWithHugeString avgt 15 112.994 ? 2.342 > ns/op > -BigDecimals.testConstructorWithLargeString avgt 15 114.016 ? 2.529 > ns/op > -BigDecimals.testConstructorWithSmallString avgt 15 19.526 ? 0.078 > ns/op > -BigDecimals.testConstructorWithString avgt 15 68.058 ? 0.836 > ns/op > > +Benchmark Mode CntScoreError > Units #bb620ba > +BigDecimals.testConstructorWithHugeString avgt 15 96.838 ? 8.743 > ns/op +16.68% > +BigDecimals.testConstructorWithLargeString avgt 15 20.904 ? 0.112 > ns/op +445.42% > +BigDecimals.testConstructorWithSmallString avgt 15 16.083 ? 0.042 > ns/op +21.40% > +BigDecimals.testConstructorWithString avgt 15 35.912 ? 0.350 > ns/op +85.91% > > > It can be seen that there is a significant performance improvement in the new > BigDecimal(Str... @wenshao I think @jddarcy means to share performance improvement in client applications, like in fastjson reading of bigdecimal from JSON, by "interesting workload the change improves". - PR Comment: https://git.openjdk.org/jdk/pull/18177#issuecomment-2080520496
Integrated: 8331114: Further improve performance of MethodTypeDesc::descriptorString
On Thu, 25 Apr 2024 09:41:11 GMT, Claes Redestad wrote: > When analyzing (startup) performance of the Classfile API I found this > opportunity to further improve `MethodTypeDescImpl::descriptorString`. > > Performance improves across the board in existing microbenchmarks: > > Name > (descString) Cnt Base ErrorTest Error Unit Change > MethodTypeDescFactories.descriptorString > (Ljava/lang/Object;Ljava/lang/String;)I 6 55,179 ± 2,027 32,920 ± 1,189 > ns/op 1,68x (p = 0,000*) > MethodTypeDescFactories.descriptorString > ()V 6 17,689 ± 1,871 11,060 ± 0,331 ns/op 1,60x (p = 0,000*) > MethodTypeDescFactories.descriptorString > ([IJLjava/lang/String;Z)Ljava/util/List; 6 86,627 ± 1,646 41,035 ± 0,636 > ns/op 2,11x (p = 0,000*) > MethodTypeDescFactories.descriptorString > ()[Ljava/lang/String; 6 18,305 ± 1,974 13,110 ± 0,089 ns/op 1,40x (p = > 0,000*) > * = significant > > > The improvement is even more pronounced when running with `-Xint`, which is > relevant for reducing startup overheads of early ClassFile API use: > > Name > (descString) Cnt Base Error Test Error Unit Change > MethodTypeDescFactories.descriptorString > (Ljava/lang/Object;Ljava/lang/String;)I 6 5122,061 ± 81,335 2626,481 ± > 101,466 ns/op 1,95x (p = 0,000*) > MethodTypeDescFactories.descriptorString > ()V 6 3481,316 ± 258,904 1489,267 ± 15,506 ns/op 2,34x (p = 0,000*) > MethodTypeDescFactories.descriptorString > ([IJLjava/lang/String;Z)Ljava/util/List; 6 7741,081 ± 1628,244 3281,778 ± > 41,892 ns/op 2,36x (p = 0,000*) > MethodTypeDescFactories.descriptorString > ()[Ljava/lang/String; 6 3677,803 ± 63,432 1495,291 ± 8,995 ns/op > 2,46x (p = 0,000*) > * = significant > ``` > > I also applied similar approach to `MethodTypeDesc::displayDescriptor`: > while not performance sensitive I think these are so inter-related that it > makes sense to implement them in a similar fashion. This pull request has now been integrated. Changeset: a078b5e6 Author:Claes Redestad URL: https://git.openjdk.org/jdk/commit/a078b5e6117d2a99386fecd349eb0e0c11b74953 Stats: 34 lines in 5 files changed: 19 ins; 1 del; 14 mod 8331114: Further improve performance of MethodTypeDesc::descriptorString Reviewed-by: mchung, liach - PR: https://git.openjdk.org/jdk/pull/18945
Re: RFR: 8331134: Port SimpleStringBuilderStrategy to use ClassFile API [v10]
On Fri, 26 Apr 2024 14:57:07 GMT, Claes Redestad wrote: >> This patch suggests a workaround to an issue with huge SCF MH expression >> trees taking excessive JIT compilation resources by reviving (part of) the >> simple bytecode-generating strategy that was originally available as an >> all-or-nothing strategy choice. >> >> Instead of reintroducing a binary strategy choice I propose a threshold >> parameter, controlled by >> `-Djava.lang.invoke.StringConcat.highArityThreshold=`: For expressions >> below or at this threshold there's no change, for expressions with an arity >> above it we use the `StringBuilder`-chain bytecode generator. >> >> There are a few trade-offs at play here which influence the choice of >> threshold. The simple high arity strategy will for example not see any reuse >> of LambdaForms but strictly always generate a class per indy callsite, which >> means we might end up with a higher total number of classes generated and >> loaded in applications if we set this value too low. It may also produce >> worse performance on average. On the other hand there is the observed >> increase in C2 resource usage as expressions grow unwieldy. On the other >> other hand high arity expressions are likely rare to begin with, with less >> opportunities for sharing than the more common low-arity expressions. >> >> I turned the submitted test case into a few JMH benchmarks and did some >> experiments with `-XX:CompileCommand=MemStat,StringConcat::concat,print`: >> >> Baseline strategy: >> 13 args: 6.3M >> 23 args: 18M >> 123 args: 868M >> >> `-Djava.lang.invoke.StringConcat.highArityThreshold=0`: >> 13 args: 2.11M >> 23 args: 3.67M >> 123 args: 4.75M >> >> For 123 args the memory overhead of the baseline strategy is 180x, but for >> 23 args we're down to a 5x memory overhead, and down to a 3x overhead for 13 >> args. Since the absolute overhead for 23 is borderline acceptable (+15Mb) >> I've conservatively chosen a threshold at arity 20. This keeps C2 resource >> pressure at a reasonable level (< 18M) while avoiding perturbing performance >> at the vast majority of call sites. >> >> I was asked to use the new class file API for mainline. There's a version of >> this patch implemented using ASM in 7c52a9f which might be a reasonable >> basis for a backport. > > Claes Redestad has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 12 commits: > > - Rebase after integration of #18953 > - Nits, clean up constant, introduce missing constant MethodTypeDesc for > toString > - Make Set.of(STRONG) a constant, fix compilation, minor code improvements > - Update > src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java > >Co-authored-by: Mandy Chung > - Minor SimpleStringBuilderStrategy:: overhead reduction > - Merge master, resolve conflicts due pr/18688 > - Dump using the hidden class name > - Pre-size stringbuilders based on constant size and a small argument factor > - @liach feedback > - Bump threshold after experiments > - ... and 2 more: https://git.openjdk.org/jdk/compare/5e2ced4b...f87fa1d7 Thanks! - PR Comment: https://git.openjdk.org/jdk/pull/18690#issuecomment-2080507738
Integrated: 8331134: Port SimpleStringBuilderStrategy to use ClassFile API
On Tue, 9 Apr 2024 12:01:49 GMT, Claes Redestad wrote: > This patch suggests a workaround to an issue with huge SCF MH expression > trees taking excessive JIT compilation resources by reviving (part of) the > simple bytecode-generating strategy that was originally available as an > all-or-nothing strategy choice. > > Instead of reintroducing a binary strategy choice I propose a threshold > parameter, controlled by > `-Djava.lang.invoke.StringConcat.highArityThreshold=`: For expressions > below or at this threshold there's no change, for expressions with an arity > above it we use the `StringBuilder`-chain bytecode generator. > > There are a few trade-offs at play here which influence the choice of > threshold. The simple high arity strategy will for example not see any reuse > of LambdaForms but strictly always generate a class per indy callsite, which > means we might end up with a higher total number of classes generated and > loaded in applications if we set this value too low. It may also produce > worse performance on average. On the other hand there is the observed > increase in C2 resource usage as expressions grow unwieldy. On the other > other hand high arity expressions are likely rare to begin with, with less > opportunities for sharing than the more common low-arity expressions. > > I turned the submitted test case into a few JMH benchmarks and did some > experiments with `-XX:CompileCommand=MemStat,StringConcat::concat,print`: > > Baseline strategy: > 13 args: 6.3M > 23 args: 18M > 123 args: 868M > > `-Djava.lang.invoke.StringConcat.highArityThreshold=0`: > 13 args: 2.11M > 23 args: 3.67M > 123 args: 4.75M > > For 123 args the memory overhead of the baseline strategy is 180x, but for 23 > args we're down to a 5x memory overhead, and down to a 3x overhead for 13 > args. Since the absolute overhead for 23 is borderline acceptable (+15Mb) > I've conservatively chosen a threshold at arity 20. This keeps C2 resource > pressure at a reasonable level (< 18M) while avoiding perturbing performance > at the vast majority of call sites. > > I was asked to use the new class file API for mainline. There's a version of > this patch implemented using ASM in 7c52a9f which might be a reasonable basis > for a backport. This pull request has now been integrated. Changeset: c3372c45 Author:Claes Redestad URL: https://git.openjdk.org/jdk/commit/c3372c455542cef2aaf673d1f14be8759bb98e8d Stats: 253 lines in 2 files changed: 80 ins; 142 del; 31 mod 8331134: Port SimpleStringBuilderStrategy to use ClassFile API Reviewed-by: mchung - PR: https://git.openjdk.org/jdk/pull/18690
Re: RFR: 8331114: Further improve performance of MethodTypeDesc::descriptorString [v5]
On Fri, 26 Apr 2024 22:27:43 GMT, Claes Redestad wrote: >> When analyzing (startup) performance of the Classfile API I found this >> opportunity to further improve `MethodTypeDescImpl::descriptorString`. >> >> Performance improves across the board in existing microbenchmarks: >> >> Name >> (descString) Cnt Base ErrorTest Error Unit Change >> MethodTypeDescFactories.descriptorString >> (Ljava/lang/Object;Ljava/lang/String;)I 6 55,179 ± 2,027 32,920 ± 1,189 >> ns/op 1,68x (p = 0,000*) >> MethodTypeDescFactories.descriptorString >> ()V 6 17,689 ± 1,871 11,060 ± 0,331 ns/op 1,60x (p = 0,000*) >> MethodTypeDescFactories.descriptorString >> ([IJLjava/lang/String;Z)Ljava/util/List; 6 86,627 ± 1,646 41,035 ± 0,636 >> ns/op 2,11x (p = 0,000*) >> MethodTypeDescFactories.descriptorString >> ()[Ljava/lang/String; 6 18,305 ± 1,974 13,110 ± 0,089 ns/op 1,40x (p = >> 0,000*) >> * = significant >> >> >> The improvement is even more pronounced when running with `-Xint`, which is >> relevant for reducing startup overheads of early ClassFile API use: >> >> Name >> (descString) Cnt Base Error Test Error Unit Change >> MethodTypeDescFactories.descriptorString >> (Ljava/lang/Object;Ljava/lang/String;)I 6 5122,061 ± 81,335 2626,481 ± >> 101,466 ns/op 1,95x (p = 0,000*) >> MethodTypeDescFactories.descriptorString >> ()V 6 3481,316 ± 258,904 1489,267 ± 15,506 ns/op 2,34x (p = 0,000*) >> MethodTypeDescFactories.descriptorString >> ([IJLjava/lang/String;Z)Ljava/util/List; 6 7741,081 ± 1628,244 3281,778 ± >> 41,892 ns/op 2,36x (p = 0,000*) >> MethodTypeDescFactories.descriptorString >> ()[Ljava/lang/String; 6 3677,803 ± 63,432 1495,291 ± 8,995 ns/op >> 2,46x (p = 0,000*) >> * = significant >> ``` >> >> I also applied similar approach to `MethodTypeDesc::displayDescriptor`: >> while not performance sensitive I think these are so inter-related that it >> makes sense to implement them in a similar fashion. > > Claes Redestad has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains eight additional > commits since the last revision: > > - Rename parameter, formatting > - Rename Wrapper.primitiveClassDescriptor -> classDescriptor > - Merge branch 'master' into methodtypedesc_descriptorString > - Minor fixes > - Use returnType field directly > - Calculate length precisely > - comma-separated > - Optimize MethodTypeDescImpl::toString Thanks! - PR Comment: https://git.openjdk.org/jdk/pull/18945#issuecomment-2080505209
Re: RFR: 8314480: Memory ordering spec updates in java.lang.ref [v28]
On Fri, 26 Apr 2024 17:59:39 GMT, Brent Christian wrote:
>> Classes in the `java.lang.ref` package would benefit from an update to bring
>> the spec in line with how the VM already behaves. The changes would focus on
>> _happens-before_ edges at some key points during reference processing.
>>
>> A couple key things we want to be able to say are:
>> - `Reference.reachabilityFence(x)` _happens-before_ reference processing
>> occurs for 'x'.
>> - `Cleaner.register()` _happens-before_ the Cleaner thread runs the
>> registered cleaning action.
>>
>> This will bring Cleaner in line (or close) with the memory visibility
>> guarantees made for finalizers in [JLS
>> 17.4.5](https://docs.oracle.com/javase/specs/jls/se18/html/jls-17.html#jls-17.4.5):
>> _"There is a happens-before edge from the end of a constructor of an object
>> to the start of a finalizer (§12.6) for that object."_
>
> Brent Christian has updated the pull request incrementally with one
> additional commit since the last revision:
>
> new section for finalizer memviz
src/java.base/share/classes/java/lang/ref/package-info.java line 137:
> 135: *
> 136: * Memory Consistency Properties
> 137: * Certain interactions between references, references queues, and the
> garbage
Suggestion:
* Certain interactions between references, reference queues, and the garbage
src/java.base/share/classes/java/lang/ref/package-info.java line 157:
> 155: * The dequeuing of a reference to a
> 156: * {@linkplain Cleaner#register(Object object, Runnable action)
> registered}
> 157: * object, by a Cleaner thread, happens-before that Cleaner
> thread runs
Are we committing to this? I.e. that there will always be a single Cleaner
thread? (i.e. one could imagine a situation where you have one polling thread
which executes the cleaning action on some other thread?)
src/java.base/share/classes/java/lang/ref/package-info.java line 176:
> 174: * Actions in a thread prior to calling
> 175: * {@link Reference#reachabilityFence Reference.reachabilityFence(x)}
> 176: * happen-before the finalizer for x is run by a finalizer
> thread.
Suggestion:
* happen-before the finalizer for {@code x} is run by a finalizer
thread.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16644#discussion_r1581818740
PR Review Comment: https://git.openjdk.org/jdk/pull/16644#discussion_r1581818931
PR Review Comment: https://git.openjdk.org/jdk/pull/16644#discussion_r1581819080
Integrated: 8296543: Update exception documentation for ExecutorService.invokeAll overriders as required
On Fri, 26 Apr 2024 08:08:25 GMT, Viktor Klang wrote: > This PR adds the exception documentation as per the ExecutorService API > contract. I also took the liberty of adding @Override-annotations to be clear > about intent. This pull request has now been integrated. Changeset: e3eb652c Author:Viktor Klang URL: https://git.openjdk.org/jdk/commit/e3eb652c251e8298c9df95d7ed2788f2cbb5f337 Stats: 31 lines in 1 file changed: 31 ins; 0 del; 0 mod 8296543: Update exception documentation for ExecutorService.invokeAll overriders as required Reviewed-by: prappo, alanb - PR: https://git.openjdk.org/jdk/pull/18966
Re: RFR: 8278255: add more warning text in ReentrantLock and ReentrantReadWriteLock [v2]
> This is an attempt to be more clear about recommendations on Lock usage. Viktor Klang has updated the pull request incrementally with one additional commit since the last revision: Update src/java.base/share/classes/java/util/concurrent/locks/ReentrantReadWriteLock.java Co-authored-by: Pavel Rappo <32523691+pavelra...@users.noreply.github.com> - Changes: - all: https://git.openjdk.org/jdk/pull/18974/files - new: https://git.openjdk.org/jdk/pull/18974/files/74c937a1..e416c59a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18974=01 - incr: https://webrevs.openjdk.org/?repo=jdk=18974=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18974.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18974/head:pull/18974 PR: https://git.openjdk.org/jdk/pull/18974
Re: RFR: 8330005: RandomGeneratorFactory.getDefault() throws exception when the runtime image only has java.base module [v4]
On Fri, 26 Apr 2024 15:24:30 GMT, Raffaello Giulietti wrote: >> Move all random generators mandated in package `java.util.random` and >> currently implemented in module `jdk.random` to module `java.base`, and >> remove module `jdk.random`. > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > Added @uses javadoc tag for j.u.r.RandomGenerator in java.base. Note that this also fixes a "bug" when calling `RandomGenerator.getDefault()` (or similar) from an unnamed module that does not have the application class loader as a(n indirect) parent. That previously resulted in the service loader only finding `Random`, `SecureRandom`, and `SplittableRandom`. As those are cached (https://github.com/openjdk/jdk/blob/fd84a28ba0aa82b2f48f247c3ec7e13acdcd6ff1/src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java#L137), even calls from somewhere else afterwards wouldn't find the correct implementation anymore. So I'm welcoming this change. - PR Comment: https://git.openjdk.org/jdk/pull/18932#issuecomment-2080464407
Re: RFR: 8330005: RandomGeneratorFactory.getDefault() throws exception when the runtime image only has java.base module [v3]
On Fri, 26 Apr 2024 13:45:39 GMT, Alan Bateman wrote:
> How useful is it to deploy with additional RandomGenerator implementations on
> the class path or module path?
It doesn't look like in its current form it's possible (without an
`--add-exports`) to have additional `java.util.random.RandomGenerator`
implementations in the class path (or module path). I tried some experiment
today to introduce a trivial `java.util.random.RandomGenerator` implementation
as follows:
package foo;
import java.util.Random;
import java.util.random.RandomGenerator;
public class DummyRandomGenerator implements RandomGenerator {
private final Random random = new Random();
@Override
public long nextLong() {
System.out.println("nextLong from " + this.getClass());
return random.nextLong();
}
}
Package this `DummyRandomGenerator` into a jar file and in that jar file
include a `META-INF/services/java.util.RandomGenerator` which points to the
`foo.DummyRandomGenerator` class. Then launch a main application with this jar
in the classpath:
import java.util.random.RandomGeneratorFactory;
public class Main {
public static void main(String[] args) {
System.out.println("available RandomGenerator(s):");
RandomGeneratorFactory.all().map(RandomGeneratorFactory::name).forEach(System.out::println);
}
}
Running this `Main` application with that jar in the classpath doesn't list the
`DummyRandomGenerator`. It appears that such application/library specific
implementations of `java.util.random.RandomGenerator` are expected to be
annotated to an annotation that belongs to the internals of the java.base
module
https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java#L391.
Changing the `DummyRandomGenerator` to have it annotated with:
@jdk.internal.util.random.RandomSupport.RandomGeneratorProperties(name="Dummy")
public class DummyRandomGenerator implements RandomGenerator {
...
and then compiling that application class with:
javac --add-exports java.base/jdk.internal.util.random=ALL-UNNAMED ...
and then re-packaging that jar with this class and rerunning the Main with that
jar in the classpath will this time return the `DummyRandomGenerator` from a
call to `RandomGeneratorFactory.all()`.
The API docs of `java.util.random.RandomGeneratorFactory` don't make a mention
of the necessity of this internal
`jdk.internal.util.random.RandomSupport.RandomGeneratorProperties` annotation.
-
PR Comment: https://git.openjdk.org/jdk/pull/18932#issuecomment-208042
Re: RFR: 8327791: Optimization for new BigDecimal(String) [v18]
On Fri, 26 Apr 2024 05:48:03 GMT, Shaojin Wen wrote:
>> The current BigDecimal(String) constructor calls String#toCharArray, which
>> has a memory allocation.
>>
>>
>> public BigDecimal(String val) {
>> this(val.toCharArray(), 0, val.length()); // allocate char[]
>> }
>>
>>
>> When the length is greater than 18, create a char[]
>>
>>
>> boolean isCompact = (len <= MAX_COMPACT_DIGITS); // 18
>> if (!isCompact) {
>> // ...
>> } else {
>> char[] coeff = new char[len]; // allocate char[]
>> // ...
>> }
>>
>>
>> This PR eliminates the two memory allocations mentioned above, resulting in
>> an approximate 60% increase in performance..
>
> Shaojin Wen has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains 22 additional
> commits since the last revision:
>
> - Merge remote-tracking branch 'upstream/master' into optim_dec_new
> - Merge remote-tracking branch 'upstream/master' into optim_dec_new
> - use while instead for
> - Update src/java.base/share/classes/java/math/BigDecimal.java
>
>Co-authored-by: Claes Redestad
> - bug fix for CharArraySequence#charAt
> - bug fix for CharArraySequence
> - fix benchmark
> - one CharArraySequence
> - restore comment
> - easier to compare
> - ... and 12 more: https://git.openjdk.org/jdk/compare/26169c54...bb620ba3
In money-related scenarios, such as product prices, etc., BigDecimal needs to
be used instead of float/double. I focus on optimizing the performance of
BigDecimal's construction and toString scenarios.
Constructing BigDecimal usually includes two scenarios:
* new BigDecimal(char[], int, int), Many libraries, such as mysql
drveri/fastjson/jackson and other commonly used libraries use this function to
construct BigDecimal
* new BigDecimal(String), There are also many scenarios, such as data
integration scenarios, scenarios where json is read from the message queue and
then converted into target database row records, often using new
BigDecimal(String)
I did a performance comparison test on Mac Book M1 Pro. The compared branches
are:
* [baseline](https://github.com/wenshao/jdk/tree/upstream_master_0312)
https://github.com/wenshao/jdk/tree/upstream_master_0312
* bb620ba [Full](https://webrevs.openjdk.org/?repo=jdk=18177=17) -
[Incremental](https://webrevs.openjdk.org/?repo=jdk=18177=16-17)
([bb620ba3](https://git.openjdk.org/jdk/pull/18177/files/bb620ba39a6f1ce17ff273bac54ebb709beb1667))
# 1. new BigDecimal(String) benchmark
Execute the following commands respectively
make test TEST="micro:java.math.BigDecimals.testConstructorWithHugeString"
make test TEST="micro:java.math.BigDecimals.testConstructorWithLargeString"
make test TEST="micro:java.math.BigDecimals.testConstructorWithSmallString"
make test TEST="micro:java.math.BigDecimals.testConstructorWithString"
## benchmark result
-Benchmark Mode CntScore Error
Units #baseline
-BigDecimals.testConstructorWithHugeString avgt 15 112.994 ? 2.342
ns/op
-BigDecimals.testConstructorWithLargeString avgt 15 114.016 ? 2.529
ns/op
-BigDecimals.testConstructorWithSmallString avgt 15 19.526 ? 0.078
ns/op
-BigDecimals.testConstructorWithString avgt 15 68.058 ? 0.836
ns/op
+Benchmark Mode CntScoreError
Units #bb620ba
+BigDecimals.testConstructorWithHugeString avgt 15 96.838 ? 8.743
ns/op +16.68%
+BigDecimals.testConstructorWithLargeString avgt 15 20.904 ? 0.112
ns/op +445.42%
+BigDecimals.testConstructorWithSmallString avgt 15 16.083 ? 0.042
ns/op +21.40%
+BigDecimals.testConstructorWithString avgt 15 35.912 ? 0.350
ns/op +85.91%
It can be seen that there is a significant performance improvement in the new
BigDecimal(String) scenario.
| case | |
|-|-|
|BigDecimals.testConstructorWithHugeString |16.68% |
|BigDecimals.testConstructorWithLargeString |445.42% |
|BigDecimals.testConstructorWithSmallString |21.40% |
|BigDecimals.testConstructorWithString |85.91% |
# 2. new BigDecima(char[],int,int) benchmark
Execute the following commands respectively
make test TEST="micro:java.math.BigDecimals.testConstructorWithHugeCharArray"
make test TEST="micro:java.math.BigDecimals.testConstructorWithLargeCharArray"
make test TEST="micro:java.math.BigDecimals.testConstructorWithSmallCharArray"
make test TEST="micro:java.math.BigDecimals.testConstructorWithCharArray"
## benchmark result
-Benchmark Mode CntScore Error
Units #baseline
-BigDecimals.testConstructorWithHugeCharArray avgt 15 94.554 ? 4.262
ns/op
-BigDecimals.testConstructorWithLargeCharArray avgt 15 94.669 ? 3.065
ns/op
-BigDecimals.testConstructorWithSmallCharArray avgt 15 16.457 ? 0.081
ns/op
