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

[Chen Liang]

On Wed, 26 Jun 2024 14:55:44 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:
> 
>   1. revert code change.
>   2. comment remove space

Changes requested by liach (Committer).

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

> 192:      *     otherwise NON_SPECIAL
> 193:      */
> 194:     private int toDecimal(byte[] str, int index, double v, FormattedFPDecimal fd) {

This `toDecimal` returns packed int with info but the other returns the new index; this is extremely confusing for future readers, especially that they have very similar signatures (difference in signature is in the middle). I recommend making this return a `long` so when it's downcasted to an int, it's just the size (like String concat's coderIndex)

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

PR Review: https://git.openjdk.org/jdk/pull/19730#pullrequestreview-2142568839
PR Review Comment: https://git.openjdk.org/jdk/pull/19730#discussion_r1655287303