On Mon, 23 Jan 2023 11:34:27 GMT, Claes Redestad <redes...@openjdk.org> wrote:
>> 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 > > Meant that you should verify that something like this, which just add a > little padding, doesn't regress with your changes: > > DateTimeFormatter dtf = new DateTimeFormatterBuilder() > .appendLiteral("Year:") > .padNext(5) > .appendValue(ChronoField.YEAR) > .toFormatter(); > ... > dtf.format(LocalDateTime.now()); > > And similar for effectively no padding (`.padNext(4)` in the above example). > As this API might often be used to ensure short 2-4 char fields are correctly > padded I think it's important that we're not adding overhead to such use > cases. Added benchmark for this ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/12131#discussion_r1147980908