On Fri, 19 Nov 2021 05:07:23 GMT, Vicente Romero <vrom...@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:
> 
>   adding the benchmark

Regarding the slot limit in `StringConcatFactory`, it's not clear to me the 
limit of 200 is normative or is merely an implementation note. The limit of 200 
slots seems to be arbitrary and shouldn't be baked into the spec. Perhaps this 
limit can be removed if the splitting logic is moved into `StringConcatFactory`.

-------------

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

Reply via email to