Delta [1] and revised [2] patches: [1] http://cr.openjdk.java.net/~bpb/8229845/webrev.00-01/ <http://cr.openjdk.java.net/~bpb/8229845/webrev.00-01/> [2] http://cr.openjdk.java.net/~bpb/8229845/webrev.01/
> On Aug 20, 2019, at 1:05 AM, Aleksey Shipilev <sh...@redhat.com> wrote: > > On 8/19/19 10:08 PM, Brian Burkhalter wrote: >> [2] http://cr.openjdk.java.net/~bpb/8229845/webrev.00/ >> <http://cr.openjdk.java.net/~bpb/8229845/webrev.00/> > > Two drive-by comments: > > *) It seems the "signum == 0" case in smallToString is regressing? Before, it > just returned the > constant-pooled "0", now it goes via StringBuilder; Yes, and I don’t see any way not to have it do so. Note that due to lines 3933-3936 in [2] this case will never be hit for values which have no more than 20 ints in their magnitude, i.e., those which do not recurse in toString(BigInteger,StringBuilder,int,int). > *) This might be "static final", while we are at it: > > 4109 private static String[] zeros = new String[64]; Above made final at L4111 in [2]. > On Aug 20, 2019, at 10:28 AM, Ivan Gerasimov <ivan.gerasi...@oracle.com> > wrote: > > While we're here. > > On 8/20/19 1:05 AM, Aleksey Shipilev wrote: >> *) This might be "static final", while we are at it: >> >> 4109 private static String[] zeros = new String[64]; >> > It may make sense to create the only string as > > private static final String zeros = "0".repeat(63); Changed at L4111 in [2]. > and then replace all > > sb.append(zeros[x]) with sb.append(zeros, 0, x) and L4006 in [2]. > sb.insert(pos, zeros[x]) with sb.insert(pos, zeros, 0, x). L4049 and L4054 in [2]. > The difference would be in one extra bounds check, and as both append() and > insert() are relatively expensive, this check would hardly be noticeable. Thanks, Brian
