RFR: 8315999: Improve Date toString performance [v9]

Claes Redestad redestad at openjdk.org
Tue Sep 12 23:37:43 UTC 2023


On Tue, 12 Sep 2023 17:23:00 GMT, 温绍锦 <duke at openjdk.org> wrote:

>> improve date toString performance, includes:
>> 
>> java.util.Date.toString
>> java.util.Date.toGMTString
>> java.time.Instant.toString
>> java.time.LocalDate.toString
>> java.time.LocalDateTime.toString
>> java.time.LocalTime.toString
>
> 温绍锦 has updated the pull request incrementally with one additional commit since the last revision:
> 
>   merge from master

I think there's still too much going on in this PR, and it should probably be split up into multiple PRs.

I think some of this is going a bit overboard. For starters I don't want to see us adding a 1000-element lookup table for "`DecimalDigits::digitTriple`" without significant evidence that that is worth it at application level. As has been pointed out elsewhere lookup tables are prone to look great on microbenchmarks but may behave poorly and even regress real world applications by competing for cache. Any further additions needs to be justified with a thorough examination and benchmarks that better simulate noisy real world applications.

src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 77:

> 75: 
> 76:     static {
> 77:         int[] digits_k = new int[1000];

I'm very skeptical that adding a 1000 element lookup table for  is worthwhile. That's a lookup table that'd span many cache lines, with plenty of cache misses. And even _if_ it is to be considered it should be split out and considered in isolation from this PR.

What does the 2, 1 or 0 value you put in the lowest byte signify?

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

Changes requested by redestad (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/15658#pullrequestreview-1623337467
PR Review Comment: https://git.openjdk.org/jdk/pull/15658#discussion_r1323702300


More information about the core-libs-dev mailing list