On Wed, 26 Jun 2024 14:55:44 GMT, Shaojin Wen <d...@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
