Reduce size of j.t.f.DateTimePrintContext::adjust

[Stephen Colebourne]

>From my perspective, the proposed change looks fine. Extracting out
error creation is a pretty common trick to get a performance gain
through additional inlining. I'd also note that it seems like the
method was identified as performance sensitive during the original
testing, as identified by the comments "early return is an
optimization".

Looking at the code now, I think the better inlining change would be
to introduce a new private method into the logic at the
common/uncommon point, something like this:

private static TemporalAccessor adjust(final TemporalAccessor
temporal, DateTimeFormatter formatter) {
  // normal case first (early return is an optimization)
  ...
  // ensure minimal change (early return is an optimization)
  ...
  adjust0(...)
}

private static TemporalAccessor adjust0(...) {
  // make adjustment
  ...
}

I suspect such a change would be more beneficial than the original proposal.

Stephen



On Tue, 5 Aug 2025 at 16:34, <jaikiran.pai at oracle.com> wrote:
>
> Hello Shaojin,
>
> Looking at the proposed change, the proposal here appears to be to replace an inline "new DateTimeException(...)" call with a call to a new private method which returns a "new DateTimeException(...)", to please the hotspot compiler's inlining decision.
>
> I think this isn't a useful change. Enabling the -XX:+PrintInlining will naturally print the runtime compiler's inlining decisions and there could be some/many logs which complain that the method couldn't be inlined. Changing the java code (like here) to closely handhold it to match the (unspecified) expectations of a particular (current) implementation of the runtime compiler shouldn't be the way to code in java language. Such changes, like the one here, won't help with the maintainability or the readability of the code.
>
> -Jaikiran
>
> On 05/08/25 6:55 am, wenshao wrote:
>
> By adding the JVM startup parameters `-XX:+UnlockDiagnosticVMOptions -XX:+PrintInlining` and analyzing the printed log information, and found that the code size of the j.t.f.DateTimePrintContext::adjust method is 382, which is greater than 325, causing inlining failure.
>
> Below is the log message:
> ```
> @ 7   java.time.format.DateTimePrintContext::adjust (382 bytes)   failed to inline: hot method too big
> ```
>
> We can extract the exception-generating code into two smaller methods, reducing the code size from 382 to 322, allowing C2 to inline the DateTimePrintContext::adjust method.
>
> The refactored code looks like this:
> ```java
>     private static TemporalAccessor adjust(final TemporalAccessor temporal, DateTimeFormatter formatter) {
>         // ...
>         if (overrideZone.normalized() instanceof ZoneOffset && temporal.isSupported(OFFSET_SECONDS) &&
>                 temporal.get(OFFSET_SECONDS) != overrideZone.getRules().getOffset(Instant.EPOCH).getTotalSeconds()) {
>             throw unableApplyOverrideZone(temporal, overrideZone);
>         }
>         // ....
>         if (f.isDateBased() && temporal.isSupported(f)) {
>             throw unableApplyOverrideChronology(temporal, overrideChrono);
>         // ...
>     }
>     private static DateTimeException unableApplyOverrideChronology(TemporalAccessor temporal, Chronology overrideChrono) {
>         return new DateTimeException("Unable to apply override chronology '" + overrideChrono +
>                 "' because the temporal object being formatted contains date fields but" +
>                 " does not represent a whole date: " + temporal);
>     }
>     private static DateTimeException unableApplyOverrideZone(TemporalAccessor temporal, ZoneId overrideZone) {
>         return new DateTimeException("Unable to apply override zone '" + overrideZone +
>                 "' because the temporal object being formatted has a different offset but" +
>                 " does not represent an instant: " + temporal);
>     }
> ```
>
> I submitted a draft Pull Request https://github.com/openjdk/jdk/pull/26633 , Hope to get your feedback.
>
> -
> Shaojin Wen