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
