RFR: 8366224: Introduce DecimalDigits.appendPair for efficient two-digit formatting and refactor DateTimeHelper [v22]

[Emanuel Peter]

On Fri, 14 Nov 2025 07:59:29 GMT, Shaojin Wen <swen at openjdk.org> wrote:

>> Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   remove JLA
>
> I wanted to modify the DecimalDigits.appendQuad method as follows, but this caused `MergeStore` to not work.
> 
>     public static void appendQuad(StringBuilder buf, int v) {
>         // The & 0x7f operation keeps the index within the safe range [0, 127] for the DIGITS array,
>         // which allows the JIT compiler to eliminate array bounds checks for performance.
>         int packed = DIGITS[(v / 100) & 0x7f] | (DIGITS[(v % 100) & 0x7f] << 16);
>         // The temporary String and byte[] objects created here are typically eliminated
>         // by the JVM's escape analysis and scalar replacement optimizations during
>         // runtime compilation, avoiding actual heap allocations in optimized code.
>         buf.append(
>                 JLA.uncheckedNewStringWithLatin1Bytes(
>                         new byte[] {(byte) packed,         (byte) (packed >> 8),
>                                     (byte) (packed >> 16), (byte) (packed  >> 24)}));
>     }
> 
> 
> The output is as follows:
> 
> [TraceMergeStores] MergePrimitiveStores::run:  868  StoreB  === 887 813 861 145  [[ 872 ]]  @byte[int:>=0] (java/lang/Cloneable,java/io/Serializable):NotNull:exact+any *, idx=7;  Memory: @byte[int:>=0] (java/lang/Cloneable,java/io/Serializable):NotNull:exact+any *, idx=7;
> [TraceMergeStores] expect no use: None
> [TraceMergeStores] expect def: None
> [TraceMergeStores] MergePrimitiveStores::run:  848  StoreB  === 888 813 840 81  [[ 853 ]]  @byte[int:>=0] (java/lang/Cloneable,java/io/Serializable):NotNull:exact+any *, idx=7;  Memory: @byte[int:>=0] (java/lang/Cloneable,java/io/Serializable):NotNull:exact+any *, idx=7;
> [TraceMergeStores] expect no use: None
> [TraceMergeStores] expect def: None
> [TraceMergeStores] MergePrimitiveStores::run:  559  StoreB  === 548 543 351 352  [[ 562 ]]  @java/lang/AbstractStringBuilder (java/lang/CharSequence,java/lang/Appendable)+16 *, name=coder, idx=13;  Memory: @java/lang/StringBuilder (java/io/Serializable,java/lang/Comparable,java/lang/CharSequence,java/lang/Appendable):NotNull:exact+16 *, name=coder, idx=13; !jvms: AbstractStringBuilder::append @ bci:78 (line 651) StringBuilder::append @ bci:2 (line 179) DecimalDigits::appendQuad @ bci:68 (line 496)
> [TraceMergeStores] expect no use: None
> [TraceMergeStores] expect def: None
> [TraceMergeStores] MergePrimitiveStores::run:  739  StoreI  === 879 813 354 456  [[ 17 ]]  @java/lang/AbstractStringBuilder (java/lang/CharSequence,java/lang/Appendable)+12 *, name=count, idx=14;  Memory: @java/lang/StringBuilder (java/io/Serializ...

@wenshao I see. I suspect that your `packed` is deconstructed into byte parts by C2 optimizations.

What happens if you route the `packed` value through some non-inlined method?


        int packed = DIGITS[(v / 100) & 0x7f] | (DIGITS[(v % 100) & 0x7f] << 16);
        packed = dontinline(packed); // prevents optimizations
        // The temporary String and byte[] objects created here are typically eliminated
        // by the JVM's escape analysis and scalar replacement optimizations during
        // runtime compilation, avoiding actual heap allocations in optimized code.
        buf.append(
                JLA.uncheckedNewStringWithLatin1Bytes(
                        new byte[] {(byte) packed,         (byte) (packed >> 8),
                                    (byte) (packed >> 16), (byte) (packed  >> 24)}));

The issue is probably that `packed >> 16)` in the lower part sees that it only requires the values from `DIGITS[(v % 100) & 0x7f] << 16`, and just redirects things, and folds away the shifts. C2 does a lot of smart things like that.

Feel free to debug this myself, and look at the C2 IR. I unfortunately have no time to dig deeper here at this time.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/26911#issuecomment-3531817491