RFR: 8278434: timeouts in test java/time/test/java/time/format/TestZoneTextPrinterParser.java
The proposed fix is to address the performance degradation caused by the fix to JDK-8275721. Some amount of the degradation cannot be avoided as the lookup now falls back up to the bundles at Locale.ROOT. However, by lowering the fallback priority of `regionFormatFallback` than `COMPAT`'s lookup, it can avoid the excess bundle lookups for regions. I also changed the test case `TestZoneTextPrinterParser.java`, which currently iterates over 3 nested loops, i.e., all-locales x all-timezones x 8, which is absolutely unnecessary. Made it to sample some locales. In addition, I added a microbenchmark for the DateFormatSymbols.getZoneStrings() method. Here is the result: Before the fix to JDK-8275721: Benchmark Mode Cnt Score Error Units ZoneStrings.testZoneStrings ss 5 6.865 ± 0.696 s/op Before the proposed fix: Benchmark Mode Cnt Score Error Units ZoneStrings.testZoneStrings ss 5 15.741 ± 4.300 s/op After the proposed fix: Benchmark Mode Cnt Score Error Units ZoneStrings.testZoneStrings ss 5 9.756 ± 3.685 s/op ------------- Commit messages: - Added a microbenchmark for zone strings - 8278434: timeouts in test java/time/test/java/time/format/TestZoneTextPrinterParser.java Changes: https://git.openjdk.java.net/jdk/pull/6790/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6790&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8278434 Stats: 72 lines in 3 files changed: 59 ins; 7 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/6790.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6790/head:pull/6790 PR: https://git.openjdk.java.net/jdk/pull/6790
The proposed fix is to address the performance degradation caused by the fix to JDK-8275721. Some amount of the degradation cannot be avoided as the lookup now falls back up to the bundles at Locale.ROOT. However, by lowering the fallback priority of `regionFormatFallback` than `COMPAT`'s lookup, it can avoid the excess bundle lookups for regions. I also changed the test case `TestZoneTextPrinterParser.java`, which currently iterates over 3 nested loops, i.e., all-locales x all-timezones x 8, which is absolutely unnecessary. Made it to sample some locales. In addition, I added a microbenchmark for the DateFormatSymbols.getZoneStrings() method. Here is the result:
Before the fix to JDK-8275721:
Benchmark Mode Cnt Score Error Units ZoneStrings.testZoneStrings ss 5 6.865 ± 0.696 s/op
Before the proposed fix:
Benchmark Mode Cnt Score Error Units ZoneStrings.testZoneStrings ss 5 15.741 ± 4.300 s/op
After the proposed fix:
Benchmark Mode Cnt Score Error Units ZoneStrings.testZoneStrings ss 5 9.756 ± 3.685 s/op
Naoto Sato has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision: - Merge branch 'master' into JDK-8278434 - Copyright year update - Added a microbenchmark for zone strings - 8278434: timeouts in test java/time/test/java/time/format/TestZoneTextPrinterParser.java ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/6790/files - new: https://git.openjdk.java.net/jdk/pull/6790/files/f14f5450..5f38bd13 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6790&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6790&range=00-01 Stats: 20434 lines in 643 files changed: 14380 ins; 3767 del; 2287 mod Patch: https://git.openjdk.java.net/jdk/pull/6790.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6790/head:pull/6790 PR: https://git.openjdk.java.net/jdk/pull/6790
On Mon, 3 Jan 2022 19:37:03 GMT, Naoto Sato <naoto@openjdk.org> wrote:
The proposed fix is to address the performance degradation caused by the fix to JDK-8275721. Some amount of the degradation cannot be avoided as the lookup now falls back up to the bundles at Locale.ROOT. However, by lowering the fallback priority of `regionFormatFallback` than `COMPAT`'s lookup, it can avoid the excess bundle lookups for regions. I also changed the test case `TestZoneTextPrinterParser.java`, which currently iterates over 3 nested loops, i.e., all-locales x all-timezones x 8, which is absolutely unnecessary. Made it to sample some locales. In addition, I added a microbenchmark for the DateFormatSymbols.getZoneStrings() method. Here is the result:
Before the fix to JDK-8275721:
Benchmark Mode Cnt Score Error Units ZoneStrings.testZoneStrings ss 5 6.865 ± 0.696 s/op
Before the proposed fix:
Benchmark Mode Cnt Score Error Units ZoneStrings.testZoneStrings ss 5 15.741 ± 4.300 s/op
After the proposed fix:
Benchmark Mode Cnt Score Error Units ZoneStrings.testZoneStrings ss 5 9.756 ± 3.685 s/op
Naoto Sato has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
- Merge branch 'master' into JDK-8278434 - Copyright year update - Added a microbenchmark for zone strings - 8278434: timeouts in test java/time/test/java/time/format/TestZoneTextPrinterParser.java
Marked as reviewed by joehw (Reviewer). It’s nice to see you’ve got most of the performance back. Hope looping through Zone strings isn't really necessary. ------------- PR: https://git.openjdk.java.net/jdk/pull/6790
On Mon, 3 Jan 2022 23:45:16 GMT, Joe Wang <joehw@openjdk.org> wrote:
Hope looping through Zone strings isn't really necessary.
Thanks, Joe. IMHO, `DateFormatSymbols.getZoneStrings()` is badly designed. It just simply exposes the names in the underlying ResourceBundle, and I cannot think of any use cases for this method, rather than using `ZoneId` or `TimeZone`'s display names retrieval methods. ------------- PR: https://git.openjdk.java.net/jdk/pull/6790
On Fri, 10 Dec 2021 00:27:25 GMT, Naoto Sato <naoto@openjdk.org> wrote:
The proposed fix is to address the performance degradation caused by the fix to JDK-8275721. Some amount of the degradation cannot be avoided as the lookup now falls back up to the bundles at Locale.ROOT. However, by lowering the fallback priority of `regionFormatFallback` than `COMPAT`'s lookup, it can avoid the excess bundle lookups for regions. I also changed the test case `TestZoneTextPrinterParser.java`, which currently iterates over 3 nested loops, i.e., all-locales x all-timezones x 8, which is absolutely unnecessary. Made it to sample some locales. In addition, I added a microbenchmark for the DateFormatSymbols.getZoneStrings() method. Here is the result:
Before the fix to JDK-8275721:
Benchmark Mode Cnt Score Error Units ZoneStrings.testZoneStrings ss 5 6.865 ± 0.696 s/op
Before the proposed fix:
Benchmark Mode Cnt Score Error Units ZoneStrings.testZoneStrings ss 5 15.741 ± 4.300 s/op
After the proposed fix:
Benchmark Mode Cnt Score Error Units ZoneStrings.testZoneStrings ss 5 9.756 ± 3.685 s/op
This pull request has now been integrated. Changeset: 8dc4437d Author: Naoto Sato <naoto@openjdk.org> URL: https://git.openjdk.java.net/jdk/commit/8dc4437d002db5d025b47f48e7420e3bae55... Stats: 73 lines in 3 files changed: 59 ins; 7 del; 7 mod 8278434: timeouts in test java/time/test/java/time/format/TestZoneTextPrinterParser.java Reviewed-by: joehw ------------- PR: https://git.openjdk.java.net/jdk/pull/6790
participants (2)
-
Joe Wang
-
Naoto Sato