RFR: 8334328: Reduce object allocation for FloatToDecimal and DoubleToDecimal [v18]

[Claes Redestad]

On Tue, 25 Jun 2024 17:29:04 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];
>>                 }
>>             ...
>
> Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision:
> 
>   from @liach: use s.getBytes for performance

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

> 45:     static final int NAN         = 5 << 8;
> 46: 
> 47:     static final byte LATIN1 = 0;

I think this somewhat unnecessarily copies names and internal implementation details from `String` (where the `coder` value is used both as a pseudo-boolean and as a shift operand, which is not applicable here). In this context we could use something like `private final boolean latin1;` (all uses of `coder` are just `if (coder == LATIN1)` so it would be simplified to `if (latin1)`)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19730#discussion_r1654812101