Re: RFR: 8261847: performace of java.lang.Record::toString should be improved [v2]
On Wed, 17 Nov 2021 22:00:30 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:
>
> addressing review comments
src/java.base/share/classes/java/lang/runtime/ObjectMethods.java line 348:
> 346: if (slots + needSlots > MAX_INDY_CONCAT_ARG_SLOTS) {
> 347: splits.add(cArgs);
> 348: cArgs = new ArrayList<>();
Maybe just `cArgs.clear();`?
-
PR: https://git.openjdk.java.net/jdk/pull/6403
Re: RFR: 8261847: performace of java.lang.Record::toString should be improved [v2]
On Wed, 17 Nov 2021 22:00:30 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: > > addressing review comments I have uploaded another iteration, some numbers for when the number of record components is pushed to the maximum (254 components): JDK18 vanilla: Before: Benchmark Mode Cnt Score Error Units MyBenchmark.base avgt4 0.670 ±0.078 ns/op MyBenchmark.equals_record avgt4 9092.649 ± 400.200 ns/op MyBenchmark.equals_value avgt4107.184 ± 15.993 ns/op MyBenchmark.record_hash_code avgt4 6284.711 ± 1985.212 ns/op MyBenchmark.record_to_string avgt4 69614.364 ± 6310.593 ns/op MyBenchmark.value_class_to_string avgt4 25910.041 ± 7822.274 ns/op MyBenchmark.value_hash_codeavgt4315.969 ± 36.545 ns/op With this PR: Benchmark Mode Cnt Score Error Units MyBenchmark.base avgt4 0.658 ±0.053 ns/op MyBenchmark.equals_record avgt4 8222.407 ± 308.440 ns/op MyBenchmark.equals_value avgt4102.627 ± 56.522 ns/op MyBenchmark.record_hash_code avgt4 6030.478 ± 243.390 ns/op MyBenchmark.record_to_string avgt4 46504.151 ± 3597.182 ns/op MyBenchmark.value_class_to_string avgt4 26196.565 ± 8154.037 ns/op MyBenchmark.value_hash_codeavgt4313.918 ±4.577 ns/op some observations, we are doing better than before but still worst than lombok. I don't think having to create a more complicated MH tree is the key, our numbers are still worst even if there is no need to create too much levels in the MH tree like for 100 record components occupying a spot each. Another observation is that even though our slowest implementation is the one for the `toString` method, in absolute terms, lombok is faster in all cases. So more work could be necessary for the rest of the methods if deemed worthy. Thanks! - PR: https://git.openjdk.java.net/jdk/pull/6403
Re: RFR: 8261847: performace of java.lang.Record::toString should be improved [v2]
> 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: addressing review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/6403/files - new: https://git.openjdk.java.net/jdk/pull/6403/files/0ccb37ab..2a2856cd Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6403=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6403=00-01 Stats: 965 lines in 2 files changed: 932 ins; 13 del; 20 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
