Re: RFR: 8261847: performace of java.lang.Record::toString should be improved [v6]

2021-11-23 Thread Vicente Romero
> Please review this PR which aims to optimize the implementation of the 
> `toString` method we provide for records. A benchmark comparing the 
> implementation we are providing for records with lombok found out that lombok 
> is much faster mainly because our implementation uses `String::format`. This 
> fix is basically delegating on StringConcatFactory::makeConcatWithConstants 
> which is faster.
> 
> TIA
> 
> This is the result of the benchmark comparing records to lombok with vanilla 
> JDK:
> 
> Benchmark  Mode  CntScoreError  Units
> MyBenchmark.base   avgt40.849 ±  0.111  ns/op
> MyBenchmark.equals_record  avgt47.343 ±  2.740  ns/op
> MyBenchmark.equals_value   avgt46.644 ±  1.920  ns/op
> MyBenchmark.record_hash_code   avgt45.763 ±  3.882  ns/op
> MyBenchmark.record_to_string   avgt4  262.626 ± 12.574  ns/op 
>   <-- Before
> MyBenchmark.value_class_to_string  avgt4   30.325 ± 21.389  ns/op
> MyBenchmark.value_hash_codeavgt45.048 ±  3.936  ns/op
> 
> 
> after this patch:
> 
> Benchmark  Mode  Cnt   Score   Error  Units
> MyBenchmark.base   avgt4   0.680 ± 0.185  ns/op
> MyBenchmark.equals_record  avgt4   5.599 ± 1.348  ns/op
> MyBenchmark.equals_value   avgt4   5.718 ± 4.633  ns/op
> MyBenchmark.record_hash_code   avgt4   4.628 ± 4.368  ns/op
> MyBenchmark.record_to_string   avgt4  26.791 ± 1.817  ns/op   
>   <--- After
> MyBenchmark.value_class_to_string  avgt4  35.473 ± 2.626  ns/op
> MyBenchmark.value_hash_codeavgt4   6.152 ± 5.101  ns/op

Vicente Romero has updated the pull request incrementally with one additional 
commit since the last revision:

  adding import at benchmark

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6403/files
  - new: https://git.openjdk.java.net/jdk/pull/6403/files/52d04ecc..0b68deb5

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6403&range=05
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6403&range=04-05

  Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6403.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6403/head:pull/6403

PR: https://git.openjdk.java.net/jdk/pull/6403


Re: RFR: 8261847: performace of java.lang.Record::toString should be improved [v6]

2021-11-23 Thread Claes Redestad
On Tue, 23 Nov 2021 15:05:47 GMT, Vicente Romero  wrote:

>> Please review this PR which aims to optimize the implementation of the 
>> `toString` method we provide for records. A benchmark comparing the 
>> implementation we are providing for records with lombok found out that 
>> lombok is much faster mainly because our implementation uses 
>> `String::format`. This fix is basically delegating on 
>> StringConcatFactory::makeConcatWithConstants which is faster.
>> 
>> TIA
>> 
>> This is the result of the benchmark comparing records to lombok with vanilla 
>> JDK:
>> 
>> Benchmark  Mode  CntScoreError  Units
>> MyBenchmark.base   avgt40.849 ±  0.111  ns/op
>> MyBenchmark.equals_record  avgt47.343 ±  2.740  ns/op
>> MyBenchmark.equals_value   avgt46.644 ±  1.920  ns/op
>> MyBenchmark.record_hash_code   avgt45.763 ±  3.882  ns/op
>> MyBenchmark.record_to_string   avgt4  262.626 ± 12.574  ns/op
>><-- Before
>> MyBenchmark.value_class_to_string  avgt4   30.325 ± 21.389  ns/op
>> MyBenchmark.value_hash_codeavgt45.048 ±  3.936  ns/op
>> 
>> 
>> after this patch:
>> 
>> Benchmark  Mode  Cnt   Score   Error  Units
>> MyBenchmark.base   avgt4   0.680 ± 0.185  ns/op
>> MyBenchmark.equals_record  avgt4   5.599 ± 1.348  ns/op
>> MyBenchmark.equals_value   avgt4   5.718 ± 4.633  ns/op
>> MyBenchmark.record_hash_code   avgt4   4.628 ± 4.368  ns/op
>> MyBenchmark.record_to_string   avgt4  26.791 ± 1.817  ns/op  
>><--- After
>> MyBenchmark.value_class_to_string  avgt4  35.473 ± 2.626  ns/op
>> MyBenchmark.value_hash_codeavgt4   6.152 ± 5.101  ns/op
>
> Vicente Romero has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   adding import at benchmark

Thanks for adding the comment and fixing typos.

-

Marked as reviewed by redestad (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/6403


Re: RFR: 8261847: performace of java.lang.Record::toString should be improved [v6]

2021-11-23 Thread Vicente Romero
On Tue, 23 Nov 2021 15:05:47 GMT, Vicente Romero  wrote:

>> Please review this PR which aims to optimize the implementation of the 
>> `toString` method we provide for records. A benchmark comparing the 
>> implementation we are providing for records with lombok found out that 
>> lombok is much faster mainly because our implementation uses 
>> `String::format`. This fix is basically delegating on 
>> StringConcatFactory::makeConcatWithConstants which is faster.
>> 
>> TIA
>> 
>> This is the result of the benchmark comparing records to lombok with vanilla 
>> JDK:
>> 
>> Benchmark  Mode  CntScoreError  Units
>> MyBenchmark.base   avgt40.849 ±  0.111  ns/op
>> MyBenchmark.equals_record  avgt47.343 ±  2.740  ns/op
>> MyBenchmark.equals_value   avgt46.644 ±  1.920  ns/op
>> MyBenchmark.record_hash_code   avgt45.763 ±  3.882  ns/op
>> MyBenchmark.record_to_string   avgt4  262.626 ± 12.574  ns/op
>><-- Before
>> MyBenchmark.value_class_to_string  avgt4   30.325 ± 21.389  ns/op
>> MyBenchmark.value_hash_codeavgt45.048 ±  3.936  ns/op
>> 
>> 
>> after this patch:
>> 
>> Benchmark  Mode  Cnt   Score   Error  Units
>> MyBenchmark.base   avgt4   0.680 ± 0.185  ns/op
>> MyBenchmark.equals_record  avgt4   5.599 ± 1.348  ns/op
>> MyBenchmark.equals_value   avgt4   5.718 ± 4.633  ns/op
>> MyBenchmark.record_hash_code   avgt4   4.628 ± 4.368  ns/op
>> MyBenchmark.record_to_string   avgt4  26.791 ± 1.817  ns/op  
>><--- After
>> MyBenchmark.value_class_to_string  avgt4  35.473 ± 2.626  ns/op
>> MyBenchmark.value_hash_codeavgt4   6.152 ± 5.101  ns/op
>
> Vicente Romero has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   adding import at benchmark

this ship is sailing! thanks everyone for the feedback and interest!

-

PR: https://git.openjdk.java.net/jdk/pull/6403