On Sun, 22 Jan 2023 09:50:21 GMT, Sergey Tsypanov <stsypa...@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