On Wed, 6 Aug 2025 12:10:52 GMT, Shaojin Wen <s...@openjdk.org> wrote:
>> src/java.base/share/classes/java/time/format/DateTimePrintContext.java line >> 152: >> >>> 150: return chrono.zonedDateTime(Instant.from(temporal), >>> overrideZone); >>> 151: } >>> 152: } >> >> Have you tested the split at different locations? I would expect line 143 or >> line 130 to perform best. There may be a case for splitting multiple times. >> You will also need a comment to indicate why the method is split. > > In theory, the smaller the method, the greater the opportunity for > performance improvement, but in DateTimeFormatterWithPaddingBench and > DateTimeFormatterBench, the improvement is the same. This depends on how often Formatters override chrono and zone, and how often these overrides are no-op so we can still fast return. if both shortcuts are significant, it may well be reasonable to set up a series of small shortcut methods and end with a "slow tail" (See the example of `ClassValue::get`, `getFromBackup`, and `getFromHashMap`). Given that example, I recommend renaming the two new split `adjust` into `adjustWithOverride` and `adjustSlow` to distinguish from the actual `adjust` method. In addition, I agree with @jodastephen that line 130 intuitively seems like the best place to break methods. Do you know if the "if have zone and instant" case is so frequent that it should be included in the 2nd method instead of the 3rd? The goal of inlining, as far as I know, is to include minimal hot code to be compiled, instead of tweaking the method size so it exactly falls into FreqInlineSize, so the less code to compile for C2 the better and faster it compiles. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/26633#discussion_r2265087021
