RFR: 8334328: Reduce object allocation for FloatToDecimal and DoubleToDecimal

[Chen Liang]

On Sat, 15 Jun 2024 01:59:42 GMT, Shaojin Wen <duke at openjdk.org> wrote:

> The current versions of FloatToDecimal and DoubleToDecimal allocate additional objects. Reducing these allocations can improve the performance of Float/Double.toString and AbstractStringBuilder's append(float/double).
> 
> This patch is just a code refactoring to reduce object allocation, but does not change the Float/Double to decimal algorithm.
> 
> The following code comments the allocated objects to be removed.
> 
> 
> class FloatToDecimal {
>     public static String toString(float v) {
>         // allocate object FloatToDecimal
>         return new FloatToDecimal().toDecimalString(v);
>     }
> 
>     public static Appendable appendTo(float v, Appendable app)
>             throws IOException {
>         // allocate object FloatToDecimal
>         return new FloatToDecimal().appendDecimalTo(v, app);
>     }
> 
>     private Appendable appendDecimalTo(float v, Appendable app)
>             throws IOException {
>         switch (toDecimal(v)) {
>             case NON_SPECIAL:
>                 // allocate object char[]
>                 char[] chars = new char[index + 1];
>                 for (int i = 0; i < chars.length; ++i) {
>                     chars[i] = (char) bytes[i];
>                 }
>                 if (app instanceof StringBuilder builder) {
>                     return builder.append(chars);
>                 }
>                 if (app instanceof StringBuffer buffer) {
>                     return buffer.append(chars);
>                 }
>                 for (char c : chars) {
>                     app.append(c);
>                 }
>                 return app;
>             // ...
>         }
>     }
> }
> 
> class DoubleToDecimal {
>     public static String toString(double v) {
>         // allocate object DoubleToDecimal
>         return new DoubleToDecimal(false).toDecimalString(v);
>     }
> 
>     public static Appendable appendTo(double v, Appendable app)
>             throws IOException {
>         // allocate object DoubleToDecimal
>         return new DoubleToDecimal(false).appendDecimalTo(v, app);
>     }
> 
>     private Appendable appendDecimalTo(double v, Appendable app)
>             throws IOException {
>         switch (toDecimal(v, null)) {
>             case NON_SPECIAL:
>                 // allocate object char[]
>                 char[] chars = new char[index + 1];
>                 for (int i = 0; i < chars.length; ++i) {
>                     chars[i] = (char) bytes[i];
>                 }
>                 if (app instanceof StringBuilder builder) {
>                     return builder.append(chars);
>                 }
>               ...

src/java.base/share/classes/java/lang/AbstractStringBuilder.java line 888:

> 886:      */
> 887:     public AbstractStringBuilder append(float f) {
> 888:         ensureCapacityInternal(count + FloatToDecimal.MAX_CHARS);

This might cause failure in edge cases where the array size reaches the limit. Might need some review on how big an impact this would be.

src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 41:

> 39: public final class DoubleToDecimal extends ToDecimal {
> 40:     public static DoubleToDecimal LATIN1 = new DoubleToDecimal(ToDecimal.LATIN1);
> 41:     public static DoubleToDecimal UTF16  = new DoubleToDecimal(ToDecimal.UTF16);

Suggestion:

    public static final DoubleToDecimal LATIN1 = new DoubleToDecimal(ToDecimal.LATIN1);
    public static final DoubleToDecimal UTF16  = new DoubleToDecimal(ToDecimal.UTF16);

For constant folding.

src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 44:

> 42: 
> 43:     /*
> 44:      * For full details about this code see the following references:

We should copy these comments to `ToDecimal`; otherwise, it's not obvious what references are being made in comments there.

src/java.base/share/classes/jdk/internal/math/ToDecimal.java line 41:

> 39: public sealed class ToDecimal permits DoubleToDecimal, FloatToDecimal{
> 40:     static final byte LATIN1 = 0;
> 41:     static final byte UTF16  = 1;

@cl4es All these string latin1 vs utf16 infrastructure are duplicates of those in StringLatin1 and StringUTF16 in java.lang. Should we move those String internals to jdk.internal packages so it's more accessible to such coder-aware special optimizations, like we did with ConstantDesc?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19730#discussion_r1640634700
PR Review Comment: https://git.openjdk.org/jdk/pull/19730#discussion_r1640634754
PR Review Comment: https://git.openjdk.org/jdk/pull/19730#discussion_r1640742561
PR Review Comment: https://git.openjdk.org/jdk/pull/19730#discussion_r1640632467