Re: RFR: 8327791: Optimization for new BigDecimal(String) [v18]

2024-04-27 Thread Chen Liang 
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&pr=18177&range=17) - 
> [Incremental](https://webrevs.openjdk.org/?repo=jdk&pr=18177&range=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

Re: RFR: 8327791: Optimization for new BigDecimal(String) [v18]

2024-04-27 Thread Shaojin Wen 
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&pr=18177&range=17) - 
[Incremental](https://webrevs.openjdk.org/?repo=jdk&pr=18177&range=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
-BigDeci

Re: RFR: 8327791: Optimization for new BigDecimal(String) [v18]

2024-04-25 Thread Shaojin Wen 
> 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/1701be36...bb620ba3

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18177/files
  - new: https://git.openjdk.org/jdk/pull/18177/files/e516b580..bb620ba3

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=18177&range=17
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18177&range=16-17

  Stats: 6763 lines in 129 files changed: 5031 ins; 1483 del; 249 mod
  Patch: https://git.openjdk.org/jdk/pull/18177.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18177/head:pull/18177

PR: https://git.openjdk.org/jdk/pull/18177