<i18n dev> RFR: 8345668: ZoneOffset.ofTotalSeconds performance regression [v3]

Chen Liang liach at openjdk.org
Sat Dec 21 00:59:42 UTC 2024


On Fri, 20 Dec 2024 21:06:16 GMT, Naoto Sato <naoto at openjdk.org> wrote:

>> The change made in [JDK-8288723](https://bugs.openjdk.org/browse/JDK-8288723) seems innocuous, but it caused this performance regression. Partially reverting the change (ones that involve `computeIfAbsent()`) to the original. Provided a benchmark that iterates the call to `ZoneOffset.ofTotalSeconds(0)` 1,000 times, which improves the operation time from 3,946ns to 2,241ns.
>
> Naoto Sato has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fixed compile error

Changes requested by liach (Reviewer).

The `putIfAbsent` remark from Roger Riggs applies to `DateTimeTextProvider` and `DecimalStyle` too.  I think reusing existing result in these two places is beneficial, as the replaced `computeIfAbsent` returns the same object identity which may be helpful for quick `equals` comparisons.

test/micro/org/openjdk/bench/java/time/ZoneOffsetBench.java line 49:

> 47:     public void ofTotalSeconds() {
> 48:         for (int i = 0; i < 1_000; i++) {
> 49:             ZoneOffset.ofTotalSeconds(0);

This benchmark method should accept a `Blackhole`, and the return value of `ofTotalSeconds` must be sent to the `Blackhole.consume` method.

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

PR Review: https://git.openjdk.org/jdk/pull/22854#pullrequestreview-2518538592
PR Comment: https://git.openjdk.org/jdk/pull/22854#issuecomment-2557926553
PR Review Comment: https://git.openjdk.org/jdk/pull/22854#discussion_r1894522864


More information about the i18n-dev mailing list