8229845: Decrease memory consumption of BigInteger.toString()

[Ivan Gerasimov]

Hi Brian.
This looks good to me, thanks!

One minor comment.  Here:
3990             if (digits > 0) {
3991                 padWithZeros(buf, digits);
3992             } else {
3993                 buf.append('0');
3994             }

I cannot really see how digits may be <= 0, so it seems to me it can be 
safely replaced by just one line `padWithZeros(buf, digits);`.

Alternatively, the entire branch `if (signum == 0) {` can be removed 
from smallToString (so that this method will only work with strictly 
positive numbers), and done only for results[1] at line 4083 because it 
is the only possible source of ZERO values in the process.


It also seems that the arithmetic in the very internal loop at 4005-4017 
can be slightly optimized, but this probably can be left for another 
enhancement.

With kind regards,
Ivan


On 8/23/19 10:11 AM, Brian Burkhalter wrote:
> Hi Ivan,
>
>> On Aug 22, 2019, at 1:24 PM, Ivan Gerasimov 
>> <ivan.gerasimov at oracle.com <mailto:ivan.gerasimov at oracle.com>> wrote:
>>
>> I have a few further suggestions how to simplify the code.
>>
>> […]
>
> Those are all good suggestions: thanks!
>
>> Here's the patch, which illustrates all the suggestions above:
>> http://cr.openjdk.java.net/~igerasim/8229845/00/webrev/
>> (I have only run the tests from test/jdk/java/math/BigInteger to 
>> verify it).
>
> This patch has a problem:
> 4023 padWithZeros(buf, digits - s.length() + (numGroups - 1) * 
> digitsPerLong[radix]);
> It is missing parentheses:
>
> 4023         padWithZeros(buf, digits - (s.length() + (numGroups - 1) 
> * digitsPerLong[radix]));
>
> I was unable to find this by debugging but implemented your 
> suggestions by hand and then found it by diff-ing the patches. My 
> updated versions with your suggestions incorporated are: [1] delta, 
> [2] absolute. I also increased the size of some of the values tested 
> at line 798 of BigIntegerTest.
>
> Thanks,
>
> Brian
>
> [1] http://cr.openjdk.java.net/~bpb/8229845/webrev.03-04/
> [2] http://cr.openjdk.java.net/~bpb/8229845/webrev.04/
>
-- 
With kind regards,
Ivan Gerasimov