<i18n dev> RFR: 8300818: Reduce complexity of padding with DateTimeFormatter [v2]

[Sergey Tsypanov]

On Sun, 22 Jan 2023 11:36:34 GMT, Claes Redestad <redestad at openjdk.org> wrote:

>> Sergey Tsypanov 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 two additional commits since the last revision:
>> 
>>  - Merge branch 'master' into dtfb
>>  - Improve padding of DateTimeFormatter
>
> src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line 2611:
> 
>> 2609:                     "Cannot print as output of " + len + " characters exceeds pad width of " + padWidth);
>> 2610:             }
>> 2611:             buf.insert(preLen, String.valueOf(padChar).repeat(padWidth - len));
> 
> Have you checked with a microbenchmark that this added allocation can be elided by JITs and that there's a significant speed-up with your changes? I don't have the necessary domain expertise to assert anything here but I suspect padding widths are typically short. Such as 2 or 4 (for day/month and year fields) so a micro should examine there's no regression for little to no padding. Unlike the original code you call `insert` even if `padWidth - len == 0`. This might be optimized away by JITs, but it'd be good to verify which is best.

The modified code is called only when a user explicitly calls `padNext(int, char)`, i.e. if I modified the example snippet as

DateTimeFormatter dtf = new DateTimeFormatterBuilder()
  .appendLiteral("Date:")
//.padNext(20, ' ')
  .append(DateTimeFormatter.ISO_DATE)
  .toFormatter();
String text = dtf.format(LocalDateTime.now());

there's no regression.

Right now I cannot build ad-hoc JDK with my changes and check it with JMH, so I'd convert this to draft until I can verify it

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

PR: https://git.openjdk.org/jdk/pull/12131