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 I think this splitting logic only works for special cases. In this case, when there is only one object acting as the source of all values. Other splitting solutions needing more inputs require multiple method handles and multiple calls because of these complexly determined limits. The immediate solution here would be to define a static final constant like MAX–STRING-CONCAT–SLOTS and leave the number unspecified. I believe 200 was chosen because it was arbitrarily safe. As Vicente and Claes have shown and I verified in the templated string work, 20-25 slots per call seems to be the sweet spot. Having the resulting method with more inputs seems to make it more difficult to JIT optimize. ------------- PR: https://git.openjdk.java.net/jdk/pull/6403
