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

[Claes Redestad]

On Sun, 22 Jan 2023 09:50:21 GMT, Sergey Tsypanov <stsypanov at openjdk.org> wrote:

>> Currently it's O(n) - we do `n` shifts of bytes within `StringBuilder`. This can be reduced to O(1) improving the code like:
>> 
>> DateTimeFormatter dtf = new DateTimeFormatterBuilder()
>>   .appendLiteral("Date:")
>>   .padNext(20, ' ')
>>   .append(DateTimeFormatter.ISO_DATE)
>>   .toFormatter();
>> String text = dtf.format(LocalDateTime.now());
>
> 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

Changes requested by redestad (Reviewer).

src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line 2603:

> 2601:         public boolean format(DateTimePrintContext context, StringBuilder buf) {
> 2602:             int preLen = buf.length();
> 2603:             if (!printerParser.format(context, buf)) {

Non-standard as it may be, this style of using `expr == false` instead of `!expr` is a style choice by the original author. I would advice against changing these piecemeal without discussion and agreement that we should consistently enforce the `!expr` style.

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.

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

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