On Sun, 11 Aug 2024 10:17:08 GMT, Shaojin Wen <d...@openjdk.org> wrote:
>> By removing the redundant code logic in >> DateTimeFormatterBuilder$InstantPrinterParser#formatTo, the codeSize can be >> reduced and the performance can be improved. > > Shaojin Wen has updated the pull request incrementally with one additional > commit since the last revision: > > copyright > I'm still skeptical of some parts of this PR as it makes the code harder to > folow. The new tests are a good addition and should be merged. > > Have you tried the performance of simply breaking out the > currentEra/beforeCurrentEra methods _without making any other changes_? Since > the logic to produce the nano fraction is going to stay in this method, I > don't really see the advantage in trying to get LocalDateTime to do the > fraction some of the time. git remote add wenshao g...@github.com:wenshao/jdk.git git fetch wenshao # baseline with test git checkout 85ee5de07dd9f45a4e1659e17f9a52e69fa77df3 make test TEST="micro:java.time.ToStringBench.instantToString" Benchmark Mode Cnt Score Error Units ToStringBench.instantToString thrpt 15 4.749 ? 0.624 ops/ms # current version git checkout 875666143e6d717634f2a3cc3c397b2ca8b63e63 make test TEST="micro:java.time.ToStringBench.instantToString" Benchmark Mode Cnt Score Error Units ToStringBench.instantToString thrpt 15 5.713 ? 0.474 ops/ms +20.29% # baseline with simply breaking out the currentEra/beforeCurrentEra methods without making any other changes git fbf66307f738cd40e061c6d26fa05c062ccd048b make test TEST="micro:java.time.ToStringBench.instantToString" Benchmark Mode Cnt Score Error Units ToStringBench.instantToString thrpt 15 4.716 ? 0.569 ops/ms -0.69% In the branch `fbf66307f738cd40e061c6d26fa05c062ccd048b`, the benchmark results are very unstable because the test data includes currentEra and beforeCurrentEra. The current version has the best performance and can handle currentEra and beforeCurrentEra The result is somewhat unexpected. I thought the performance improvement was mainly due to codeSize < 325. The specific reason needs further analysis. ------------- PR Comment: https://git.openjdk.org/jdk/pull/20353#issuecomment-2282722011