On Wed, 12 Nov 2025 15:01:49 GMT, Shaojin Wen <[email protected]> wrote:
>> This PR introduces a new efficient API for appending two-digit integers to
>> StringBuilders and refactors DateTimeHelper to leverage this new
>> functionality.
>>
>> Changes include:
>>
>> 1. New `appendPair` method for efficient two-digit integer formatting
>> (00-99):
>> - Added `AbstractStringBuilder.appendLatin1(char c1, char c2)` with core
>> implementation
>> - Added `JavaLangAccess.appendPair(StringBuilder, char c1, char c2)` for
>> internal access
>> - Added `DecimalDigits.appendPair(StringBuilder, int)` public static
>> utility method
>> - Enhanced Javadoc documentation for all new methods
>>
>> 2. Refactored `DateTimeHelper` to use the new `DecimalDigits.appendPair`:
>> - Updated `DateTimeHelper.formatTo` methods for `LocalDate` and
>> `LocalTime`
>> - Replaced manual formatting logic with the new efficient two-digit
>> appending
>> - Improved code clarity and consistency in date/time formatting
>>
>> These changes improve code clarity and performance when formatting two-digit
>> numbers, particularly in date/time formatting scenarios.
>
> Shaojin Wen has updated the pull request incrementally with one additional
> commit since the last revision:
>
> static field JLA
src/java.base/share/classes/java/time/MonthDay.java line 753:
> 751: @Override
> 752: public String toString() {
> 753: StringBuilder buf = new StringBuilder(10);
Why is this set to 10 instead of 7?
src/java.base/share/classes/java/time/YearMonth.java line 1201:
> 1199: public String toString() {
> 1200: int absYear = Math.abs(year);
> 1201: StringBuilder buf = new StringBuilder(9);
I think 7 is better for most cases, right?
src/java.base/share/classes/java/time/ZoneOffset.java line 466:
> 464: } else {
> 465: int absTotalSeconds = Math.abs(totalSeconds);
> 466: StringBuilder buf = new StringBuilder();
This can be pre-sized to 9 (or 7 if seconds is 0)
src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 463:
> 461: */
> 462: public static void appendPair(StringBuilder buf, int v) {
> 463: int packed = DIGITS[v & 0x7f];
We should assert v is in range so errors can be caught when we run tests. Same
for appendQuad.
src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 464:
> 462: public static void appendPair(StringBuilder buf, int v) {
> 463: int packed = DIGITS[v & 0x7f];
> 464: buf.append(
Add a comment in the middle, that compiler should be able to eliminate the
string and array allocation here. Same for appendQuad.
src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 466:
> 464: buf.append(
> 465: JLA.uncheckedNewStringWithLatin1Bytes(
> 466: new byte[] {(byte) (packed & 0xFF), (byte)
> (packed >> 8)}));
Suggestion:
new byte[] {(byte) packed, (byte) (packed >> 8)}));
src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 484:
> 482: */
> 483: public static void appendQuad(StringBuilder buf, int v) {
> 484: int y01 = v / 100;
Same assert bound check request
src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 486:
> 484: int y01 = v / 100;
> 485: int packed = DIGITS[y01 & 0x7f] | (DIGITS[(v - y01 * 100) &
> 0x7f] << 16);
> 486: buf.append(
Same comment about noting the compiler can eliminate array and string
allocations.
src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 488:
> 486: buf.append(
> 487: JLA.uncheckedNewStringWithLatin1Bytes(
> 488: new byte[] {(byte) (packed & 0xFF), (byte)
> (packed >> 8), (byte) (packed >> 16), (byte) (packed >> 24)}));
Suggestion:
new byte[] {(byte) packed, (byte) (packed >> 8), (byte)
(packed >> 16), (byte) (packed >> 24)}));
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26911#discussion_r2520394779
PR Review Comment: https://git.openjdk.org/jdk/pull/26911#discussion_r2520398192
PR Review Comment: https://git.openjdk.org/jdk/pull/26911#discussion_r2520403542
PR Review Comment: https://git.openjdk.org/jdk/pull/26911#discussion_r2520418117
PR Review Comment: https://git.openjdk.org/jdk/pull/26911#discussion_r2520428744
PR Review Comment: https://git.openjdk.org/jdk/pull/26911#discussion_r2520408664
PR Review Comment: https://git.openjdk.org/jdk/pull/26911#discussion_r2520427212
PR Review Comment: https://git.openjdk.org/jdk/pull/26911#discussion_r2520429334
PR Review Comment: https://git.openjdk.org/jdk/pull/26911#discussion_r2520418300
