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

[Vicente Romero]

On Wed, 17 Nov 2021 22:00:30 GMT, Vicente Romero <vromero at openjdk.org> 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  Cnt    Score    Error  Units
>> MyBenchmark.base                   avgt    4    0.849 ±  0.111  ns/op
>> MyBenchmark.equals_record          avgt    4    7.343 ±  2.740  ns/op
>> MyBenchmark.equals_value           avgt    4    6.644 ±  1.920  ns/op
>> MyBenchmark.record_hash_code       avgt    4    5.763 ±  3.882  ns/op
>> MyBenchmark.record_to_string       avgt    4  262.626 ± 12.574  ns/op               <------ Before
>> MyBenchmark.value_class_to_string  avgt    4   30.325 ± 21.389  ns/op
>> MyBenchmark.value_hash_code        avgt    4    5.048 ±  3.936  ns/op
>> 
>> 
>> after this patch:
>> 
>> Benchmark                          Mode  Cnt   Score   Error  Units
>> MyBenchmark.base                   avgt    4   0.680 ± 0.185  ns/op
>> MyBenchmark.equals_record          avgt    4   5.599 ± 1.348  ns/op
>> MyBenchmark.equals_value           avgt    4   5.718 ± 4.633  ns/op
>> MyBenchmark.record_hash_code       avgt    4   4.628 ± 4.368  ns/op
>> MyBenchmark.record_to_string       avgt    4  26.791 ± 1.817  ns/op                 <------- After
>> MyBenchmark.value_class_to_string  avgt    4  35.473 ± 2.626  ns/op
>> MyBenchmark.value_hash_code        avgt    4   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                   avgt    4      0.670 ±    0.078  ns/op
MyBenchmark.equals_record          avgt    4   9092.649 ±  400.200  ns/op
MyBenchmark.equals_value           avgt    4    107.184 ±   15.993  ns/op
MyBenchmark.record_hash_code       avgt    4   6284.711 ± 1985.212  ns/op
MyBenchmark.record_to_string       avgt    4  69614.364 ± 6310.593  ns/op
MyBenchmark.value_class_to_string  avgt    4  25910.041 ± 7822.274  ns/op
MyBenchmark.value_hash_code        avgt    4    315.969 ±   36.545  ns/op


With this PR:

Benchmark                          Mode  Cnt      Score      Error  Units
MyBenchmark.base                   avgt    4      0.658 ±    0.053  ns/op
MyBenchmark.equals_record          avgt    4   8222.407 ±  308.440  ns/op
MyBenchmark.equals_value           avgt    4    102.627 ±   56.522  ns/op
MyBenchmark.record_hash_code       avgt    4   6030.478 ±  243.390  ns/op
MyBenchmark.record_to_string       avgt    4  46504.151 ± 3597.182  ns/op
MyBenchmark.value_class_to_string  avgt    4  26196.565 ± 8154.037  ns/op
MyBenchmark.value_hash_code        avgt    4    313.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