<div class="__aliyun_email_body_block"><div  style="font-family: Tahoma, Arial, STHeitiSC-Light, SimSun"><div  style="clear: both; font-family: Tahoma, Arial, STHeitiSC-Light, SimSun;"><span  style="font-family: Tahoma, Arial, STHeitiSC-Light, SimSun;"><span >Thank you, Stephen, for your feedback.</span></span></div><div  style="clear: both; font-family: Tahoma, Arial, STHeitiSC-Light, SimSun;"><br ><div  style="clear: both;">Your suggestion is better; it will make the adjust method smaller (codeSize 143), allowing for more inlining within the same inline root.</div><div  style="clear: both;"><br ></div><div ><span >I've made changes based on your suggestions in the draft pull request (<a  href="https://github.com/openjdk/jdk/pull/26633" target="_blank">https://github.com/openjdk/jdk/pull/26633</a>). Could you please review this common/uncommon split and see if it's correct?</span></div><span  style="font-family: Tahoma, Arial, STHeitiSC-Light, SimSun;"><div  style="clear: both; font-family: Tahoma, Arial, STHeitiSC-Light, SimSun;"><span  style="font-family: Tahoma, Arial, STHeitiSC-Light, SimSun;"><br ></span></div>-</span></div><div  style="clear: both; font-family: Tahoma, Arial, STHeitiSC-Light, SimSun;"><span  style="font-family: Tahoma, Arial, STHeitiSC-Light, SimSun;">Shaojin Wen</span></div><div  style="clear: both; font-family: Tahoma, Arial, STHeitiSC-Light, SimSun;"><span  style="font-family: Tahoma, Arial, STHeitiSC-Light, SimSun;"><br ></span></div><blockquote  _quote="1" style="margin-right: 0px; margin-top: 0px; margin-bottom: 0px; font-family: Tahoma, Arial, STHeiti, SimSun; font-size: 14px; color: rgb(0, 0, 0);"><div  class="alimail-quote"><div  style="clear: both;">------------------------------------------------------------------</div><div  style="clear: both;">发件人：Stephen Colebourne <scolebourne@joda.org></div><div  style="clear: both;">发送时间：2025年8月6日(周三) 05:16</div><div  style="clear: both;">收件人："core-libs-dev"<core-libs-dev@openjdk.org></div><div  style="clear: both;">主　题：Re: Reduce size of j.t.f.DateTimePrintContext::adjust</div><div  style="clear: both;"><br ></div>From my perspective, the proposed change looks fine. Extracting out<br >error creation is a pretty common trick to get a performance gain<br >through additional inlining. I'd also note that it seems like the<br >method was identified as performance sensitive during the original<br >testing, as identified by the comments "early return is an<br >optimization".<br ><br >Looking at the code now, I think the better inlining change would be<br >to introduce a new private method into the logic at the<br >common/uncommon point, something like this:<br ><br >private static TemporalAccessor adjust(final TemporalAccessor<br >temporal, DateTimeFormatter formatter) {<br >  // normal case first (early return is an optimization)<br >  ...<br >  // ensure minimal change (early return is an optimization)<br >  ...<br >  adjust0(...)<br >}<br ><br >private static TemporalAccessor adjust0(...) {<br >  // make adjustment<br >  ...<br >}<br ><br >I suspect such a change would be more beneficial than the original proposal.<br ><br >Stephen<br ><br ><br ><br >On Tue, 5 Aug 2025 at 16:34, <jaikiran.pai@oracle.com> wrote:<br >><br >> Hello Shaojin,<br >><br >> 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.<br >><br >> 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.<br >><br >> -Jaikiran<br >><br >> On 05/08/25 6:55 am, wenshao wrote:<br >><br >> 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.<br >><br >> Below is the log message:<br >> ```<br >> @ 7   java.time.format.DateTimePrintContext::adjust (382 bytes)   failed to inline: hot method too big<br >> ```<br >><br >> 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.<br >><br >> The refactored code looks like this:<br >> ```java<br >>     private static TemporalAccessor adjust(final TemporalAccessor temporal, DateTimeFormatter formatter) {<br >>         // ...<br >>         if (overrideZone.normalized() instanceof ZoneOffset && temporal.isSupported(OFFSET_SECONDS) &&<br >>                 temporal.get(OFFSET_SECONDS) != overrideZone.getRules().getOffset(Instant.EPOCH).getTotalSeconds()) {<br >>             throw unableApplyOverrideZone(temporal, overrideZone);<br >>         }<br >>         // ....<br >>         if (f.isDateBased() && temporal.isSupported(f)) {<br >>             throw unableApplyOverrideChronology(temporal, overrideChrono);<br >>         // ...<br >>     }<br >>     private static DateTimeException unableApplyOverrideChronology(TemporalAccessor temporal, Chronology overrideChrono) {<br >>         return new DateTimeException("Unable to apply override chronology '" + overrideChrono +<br >>                 "' because the temporal object being formatted contains date fields but" +<br >>                 " does not represent a whole date: " + temporal);<br >>     }<br >>     private static DateTimeException unableApplyOverrideZone(TemporalAccessor temporal, ZoneId overrideZone) {<br >>         return new DateTimeException("Unable to apply override zone '" + overrideZone +<br >>                 "' because the temporal object being formatted has a different offset but" +<br >>                 " does not represent an instant: " + temporal);<br >>     }<br >> ```<br >><br >> I submitted a draft Pull Request <a  href="https://github.com/openjdk/jdk/pull/26633" target="_blank">https://github.com/openjdk/jdk/pull/26633</a> , Hope to get your feedback.<br >><br >> -<br >> Shaojin Wen</div></blockquote><div  style="line-height: 20px; clear: both;"><br ></div></div></div>