Re: [PATCH] Use StringJoiner where appropriate in java.base

2019-06-21 Thread Сергей Цыпанов 
> Yes, StringJoiner is really good at joining Strings (hence the name).
> But it is not very good at joining primitives. Because you have to
> convert each primitive to a String before you can add it.
> StringBuilder does not need to allocate a String when you add a
> primitive.

I've reworked my patch to affect only the spots where we either already have 
Strings 
or do convertion to String (just like StringBuilder.append(Object) does using 
String::valueOf).

Also I've dropped erroneous changes to Atomic*Array (thanks Martin for spotting 
this).

Regards,
Sergey Tsypanovdiff --git a/src/java.base/share/classes/java/util/Arrays.java b/src/java.base/share/classes/java/util/Arrays.java
--- a/src/java.base/share/classes/java/util/Arrays.java
+++ b/src/java.base/share/classes/java/util/Arrays.java
@@ -5123,19 +5123,14 @@
 public static String toString(Object[] a) {
 if (a == null)
 return "null";
-
-int iMax = a.length - 1;
-if (iMax == -1)
+if (a.length == 0)
 return "[]";
 
-StringBuilder b = new StringBuilder();
-b.append('[');
-for (int i = 0; ; i++) {
-b.append(String.valueOf(a[i]));
-if (i == iMax)
-return b.append(']').toString();
-b.append(", ");
+StringJoiner sj = new StringJoiner(", ", "[", "]");
+for (Object o : a) {
+sj.add(String.valueOf(o));
 }
+return sj.toString();
 }
 
 /**
diff --git a/src/java.base/share/classes/sun/reflect/annotation/AnnotationInvocationHandler.java b/src/java.base/share/classes/sun/reflect/annotation/AnnotationInvocationHandler.java
--- a/src/java.base/share/classes/sun/reflect/annotation/AnnotationInvocationHandler.java
+++ b/src/java.base/share/classes/sun/reflect/annotation/AnnotationInvocationHandler.java
@@ -309,12 +309,10 @@
  * of an annotation.
  */
 private static String toSourceString(String s) {
-StringBuilder sb = new StringBuilder();
-sb.append('"');
+StringJoiner sb = new StringJoiner("", "\"", "\"");
 for (int i = 0; i < s.length(); i++) {
-sb.append(quote(s.charAt(i)));
+sb.add(quote(s.charAt(i)));
 }
-sb.append('"');
 return sb.toString();
 }
 
diff --git a/src/java.base/share/classes/sun/security/pkcs/SigningCertificateInfo.java b/src/java.base/share/classes/sun/security/pkcs/SigningCertificateInfo.java
--- a/src/java.base/share/classes/sun/security/pkcs/SigningCertificateInfo.java
+++ b/src/java.base/share/classes/sun/security/pkcs/SigningCertificateInfo.java
@@ -26,10 +26,9 @@
 package sun.security.pkcs;
 
 import java.io.IOException;
-import java.util.ArrayList;
+import java.util.StringJoiner;
 
 import sun.security.util.HexDumpEncoder;
-import sun.security.util.DerInputStream;
 import sun.security.util.DerValue;
 import sun.security.x509.GeneralNames;
 import sun.security.x509.SerialNumber;
@@ -92,14 +91,10 @@
 }
 
 public String toString() {
-StringBuilder sb = new StringBuilder();
-sb.append("[\n");
-for (int i = 0; i < certId.length; i++) {
-sb.append(certId[i].toString());
+StringJoiner sb = new StringJoiner("", "[\n", "\n]");
+for (ESSCertId id : certId) {
+sb.add(id.toString());
 }
-// format policies as a string
-sb.append("\n]");
-
 return sb.toString();
 }
 
diff --git a/src/java.xml/share/classes/com/sun/org/apache/bcel/internal/classfile/ArrayElementValue.java b/src/java.xml/share/classes/com/sun/org/apache/bcel/internal/classfile/ArrayElementValue.java
--- a/src/java.xml/share/classes/com/sun/org/apache/bcel/internal/classfile/ArrayElementValue.java
+++ b/src/java.xml/share/classes/com/sun/org/apache/bcel/internal/classfile/ArrayElementValue.java
@@ -23,6 +23,7 @@
 
 import java.io.DataOutputStream;
 import java.io.IOException;
+import java.util.StringJoiner;
 
 /**
  * @since 6.0
@@ -35,16 +36,10 @@
 @Override
 public String toString()
 {
-final StringBuilder sb = new StringBuilder();
-sb.append("{");
-for (int i = 0; i < evalues.length; i++)
-{
-sb.append(evalues[i]);
-if ((i + 1) < evalues.length) {
-sb.append(",");
-}
+StringJoiner sb = new StringJoiner(",", "{", "}");
+for (ElementValue evalue : evalues) {
+sb.add(evalue.stringifyValue());
 }
-sb.append("}");
 return sb.toString();
 }
 
@@ -71,16 +66,10 @@
 @Override
 public String stringifyValue()
 {
-final StringBuilder sb = new StringBuilder();
-sb.append("[");
-for (int i = 0; i < evalues.length; i++)
-{
-sb.append(evalues[i].stringifyValue());
-if ((i + 1) < evalues.length) {
-sb.append(",");
-}
+StringJoiner sb = new StringJoiner(",", "[",

Re: [PATCH] Use StringJoiner where appropriate in java.base

2019-06-21 Thread Peter Levart 




On 6/21/19 9:41 AM, Andrew Haley wrote:

On 6/20/19 9:31 PM, Peter Levart wrote:


I would also add overflow checks when computing the length of
resulting byte[]. First I would pre-check the length of passed in
int[] array (it must be less than Integer.MAX_VALUE / 3), then
checking for negative size after each addition of element length,
throwing OOME if overflow happens.

OutOfMemoryException? Are you sure? The system isn't out of memory or
any other resource, it's just that the arguments are too large. Also,
it might be cleaner to use addExact().



StringBuilder throws OutOfMemoryError when appending over its maximum 
capacity, so I thought this would keep that behavior.


Regards, Peter

Re: [PATCH] Use StringJoiner where appropriate in java.base

2019-06-21 Thread Andrew Haley 
On 6/20/19 9:31 PM, Peter Levart wrote:

> I would also add overflow checks when computing the length of
> resulting byte[]. First I would pre-check the length of passed in
> int[] array (it must be less than Integer.MAX_VALUE / 3), then
> checking for negative size after each addition of element length,
> throwing OOME if overflow happens.

OutOfMemoryException? Are you sure? The system isn't out of memory or
any other resource, it's just that the arguments are too large. Also,
it might be cleaner to use addExact().

-- 
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. 
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

Re: [PATCH] Use StringJoiner where appropriate in java.base

2019-06-21 Thread Kasper Nielsen 
On Thu, 20 Jun 2019 at 21:31, Peter Levart  wrote:
>
>
> On 6/20/19 2:39 PM, Kasper Nielsen wrote:
> >> If you allowed a two-time pass of the primitive array you could
> >> actually create a version that only allocated the actual String and
> >> backing array.
> > I tried implementing it
> > https://gist.github.com/kaspernielsen/62e4eedffdb395228777925551a45e7f
> > And got a 30-40 % performance increase.
>
> This is nice and garbage-free.
>
> This method is not very performance critical I think (unless someone is
> using it to format int[] into JSON for example), so I don't know whether
> optimizing it is that important. OTOH the optimized code is not that
> difficult to understand, so why not? I would also add overflow checks
> when computing the length of resulting byte[]. First I would pre-check
> the length of passed in int[] array (it must be less than
> Integer.MAX_VALUE / 3), then checking for negative size after each
> addition of element length, throwing OOME if overflow happens. Then I
> would re-check if the performance is still favorable and see if the
> resulting method is not too complicated to understand after all.

You actually don't need to check on each element. Use a long instead of an
int to sum up all sizes. And then just check once, which should not effect
performance.

But implementing it do require adding a public method to (preferable) every
primitive wrapper. As Arrays is in java.util and needs access to package
private methods (stringSize() and getChars()) in each wrapper. One way to do
it would be to add:

class Integer
   public static String toArrayString(int... ints)

I do actually occasionally miss such a method with varargs sometimes.
I also think I would let the method throw an NPE on null instead of
returning "null".
And change the implementation of Arrays.toString to

public static String toString(int[] a) {
  if (a == null)
return "null";
  return Integer.toStringArray(a);
}

/Kasper

Re: [PATCH] Use StringJoiner where appropriate in java.base

2019-06-20 Thread Peter Levart 



On 6/20/19 2:39 PM, Kasper Nielsen wrote:

If you allowed a two-time pass of the primitive array you could
actually create a version that only allocated the actual String and
backing array.

I tried implementing it
https://gist.github.com/kaspernielsen/62e4eedffdb395228777925551a45e7f
And got a 30-40 % performance increase.


This is nice and garbage-free.

This method is not very performance critical I think (unless someone is 
using it to format int[] into JSON for example), so I don't know whether 
optimizing it is that important. OTOH the optimized code is not that 
difficult to understand, so why not? I would also add overflow checks 
when computing the length of resulting byte[]. First I would pre-check 
the length of passed in int[] array (it must be less than 
Integer.MAX_VALUE / 3), then checking for negative size after each 
addition of element length, throwing OOME if overflow happens. Then I 
would re-check if the performance is still favorable and see if the 
resulting method is not too complicated to understand after all.


Regards, Peter




ToString2.toStringExisting1  avgt5 16.855 ±0.960  ns/op
ToString2.toStringExisting   10  avgt5 79.247 ±3.142  ns/op
ToString2.toStringExisting  100  avgt5814.197 ±   46.062  ns/op
ToString2.toStringExisting 1000  avgt5  15288.172 ± 1649.338  ns/op

ToString2.toString2Pass   1  avgt5 13.671 ±0.142  ns/op
ToString2.toString2Pass  10  avgt5 54.090 ±0.724  ns/op
ToString2.toString2Pass 100  avgt5513.508 ±6.063  ns/op
ToString2.toString2Pass1000  avgt5   9189.950 ±   47.059  ns/op

ToString2.toStringExisting:·gc.alloc.rate.norm 1  avgt
5 80.000 ±0.001B/op
ToString2.toStringExisting:·gc.alloc.rate.norm10  avgt
5160.000 ±0.001B/op
ToString2.toStringExisting:·gc.alloc.rate.norm   100  avgt
5   1664.000 ±0.001B/op
ToString2.toStringExisting:·gc.alloc.rate.norm  1000  avgt
5  23536.006 ±0.001B/op

ToString2.toString2Pass:·gc.alloc.rate.norm1  avgt
5 48.000 ±0.001B/op
ToString2.toString2Pass:·gc.alloc.rate.norm   10  avgt
5120.000 ±0.001B/op
ToString2.toString2Pass:·gc.alloc.rate.norm  100  avgt
5840.000 ±0.001B/op
ToString2.toString2Pass:·gc.alloc.rate.norm 1000  avgt
5   9848.004 ±0.001B/op

Don't know if it is something worth adding?

/Kasper

Re: [PATCH] Use StringJoiner where appropriate in java.base

2019-06-20 Thread Kasper Nielsen 
On Thu, 20 Jun 2019 at 13:53, Сергей Цыпанов  wrote:
>
> On JDK 11 I've got the following results
>
> https://github.com/stsypanov/strings/blob/master/results/StringBuilderVsStringJoinerBenchmark.txt
>
> In other words StringJoiner wins when we have longer Strings or more of them 
> because we don't need to reallocate underlying array of StringBuilder and 
> check it's bounds when appending. Instead we can allocate storage only once 
> in StringJoine::toString.

Hi,

Yes, StringJoiner is really good at joining Strings (hence the name).
But it is not very good at joining primitives. Because you have to
convert each primitive to a String before you can add it.
StringBuilder does not need to allocate a String when you add a
primitive.

Try running the same test but this time with int[] instead of
String[]. And I'm 100% sure that StringBuilder will be fastest.

/Kasper

Re: [PATCH] Use StringJoiner where appropriate in java.base

2019-06-20 Thread Сергей Цыпанов 
Hello,

pay attention that results also depend on String length: e.g. consider this 
benchmark 
https://github.com/stsypanov/strings/blob/master/src/main/java/tsypanov/strings/benchmark/string/StringBuilderVsStringJoinerBenchmark.java

On JDK 11 I've got the following results

https://github.com/stsypanov/strings/blob/master/results/StringBuilderVsStringJoinerBenchmark.txt

In other words StringJoiner wins when we have longer Strings or more of them 
because we don't need to reallocate underlying array of StringBuilder and check 
it's bounds when appending. Instead we can allocate storage only once in 
StringJoine::toString.

With best regards,
Sergey Tsypanov


20.06.2019, 13:17, "Kasper Nielsen" :
> On Thu, 20 Jun 2019 at 11:55, Peter Levart  wrote:
>>  Hi,
>>
>>  On 6/20/19 10:50 AM, Kasper Nielsen wrote:
>>  > Hi,
>>  >
>>  > On Wed, 19 Jun 2019 at 14:12, Сергей Цыпанов  
>> wrote:
>>  >> Hello,
>>  >>
>>  >> in JDK code base we have many places (mainly in j.u.Arrays) where we 
>> convert array to String using verbose constructions with StringBuilder.
>>  >>
>>  >> As far as we have got StringJoiner for a long time we can use it making 
>> the code more simple.
>>  > It may make the code simpler, but it also comes with a hefty
>>  > performance penalty, taking twice as long in most cases compared to
>>  > the existing code.
>>  >
>>  > A quick benchmark toString'ing int arrays of size 1,10,100,1000
>>  >
>>  > Benchmark (size) Mode Cnt Score Error Units
>>  > ToString2.toStringExisting 1 avgt 5 16.675 ± 0.327 ns/op
>>  > ToString2.toStringExisting 10 avgt 5 78.467 ± 1.373 ns/op
>>  > ToString2.toStringExisting 100 avgt 5 801.956 ± 7.517 ns/op
>>  > ToString2.toStringExisting 1000 avgt 5 14944.235 ± 155.393 ns/op
>>  >
>>  > ToString2.toStringSuggested 1 avgt 5 35.053 ± 0.533 ns/op
>>  > ToString2.toStringSuggested 10 avgt 5 222.043 ± 10.157 ns/op
>>  > ToString2.toStringSuggested 100 avgt 5 2150.948 ± 13.285 ns/op
>>  > ToString2.toStringSuggested 1000 avgt 5 23411.264 ± 201.721 ns/op
>>  >
>>  > /Kasper
>>
>>  StringJoiner may be faster if you already have String objects at hand,
>>  since it is able to exactly pre-size the target array and need not
>>  copy/resize it later, but if you append primitive types, then creating
>>  intermediate String objects referenced from a data structure - the
>>  StringJointer (so they can not be scalarized by JIT) is contra-productive.
>>
>>  An interesting test would be to run Kasper's JMH benchmark with "-prof
>>  gc" option. I think it will show more garbage created per call too.
>
> Yes, less garbage with StringBuilder (existing)
>
> ToString2.toStringExisting:·gc.alloc.rate.norm 1 avgt
>    5 80.000 ± 0.001 B/op
> ToString2.toStringExisting:·gc.alloc.rate.norm 10 avgt
>    5 160.000 ± 0.001 B/op
> ToString2.toStringExisting:·gc.alloc.rate.norm 100 avgt
>    5 1664.000 ± 0.001 B/op
> ToString2.toStringExisting:·gc.alloc.rate.norm 1000 avgt
>    5 23536.006 ± 0.001 B/op
>
> ToString2.toStringSuggested:·gc.alloc.rate.norm 1 avgt
>    5 168.000 ± 0.001 B/op
> ToString2.toStringSuggested:·gc.alloc.rate.norm 10 avgt
>    5 760.000 ± 0.001 B/op
> ToString2.toStringSuggested:·gc.alloc.rate.norm 100 avgt
>    5 7144.001 ± 0.001 B/op
> ToString2.toStringSuggested:·gc.alloc.rate.norm 1000 avgt
>    5 71024.010 ± 0.001 B/op
>
> If you allowed a two-time pass of the primitive array you could
> actually create a version that only allocated the actual String and
> backing array.
> But don't know if it is worth it. But then again I'm always surprised
> by the amount of string processing being done.
>
> /Kasper

Re: [PATCH] Use StringJoiner where appropriate in java.base

2019-06-20 Thread Kasper Nielsen 
> If you allowed a two-time pass of the primitive array you could
> actually create a version that only allocated the actual String and
> backing array.

I tried implementing it
https://gist.github.com/kaspernielsen/62e4eedffdb395228777925551a45e7f
And got a 30-40 % performance increase.

ToString2.toStringExisting1  avgt5 16.855 ±0.960  ns/op
ToString2.toStringExisting   10  avgt5 79.247 ±3.142  ns/op
ToString2.toStringExisting  100  avgt5814.197 ±   46.062  ns/op
ToString2.toStringExisting 1000  avgt5  15288.172 ± 1649.338  ns/op

ToString2.toString2Pass   1  avgt5 13.671 ±0.142  ns/op
ToString2.toString2Pass  10  avgt5 54.090 ±0.724  ns/op
ToString2.toString2Pass 100  avgt5513.508 ±6.063  ns/op
ToString2.toString2Pass1000  avgt5   9189.950 ±   47.059  ns/op

ToString2.toStringExisting:·gc.alloc.rate.norm 1  avgt
   5 80.000 ±0.001B/op
ToString2.toStringExisting:·gc.alloc.rate.norm10  avgt
   5160.000 ±0.001B/op
ToString2.toStringExisting:·gc.alloc.rate.norm   100  avgt
   5   1664.000 ±0.001B/op
ToString2.toStringExisting:·gc.alloc.rate.norm  1000  avgt
   5  23536.006 ±0.001B/op

ToString2.toString2Pass:·gc.alloc.rate.norm1  avgt
5 48.000 ±0.001B/op
ToString2.toString2Pass:·gc.alloc.rate.norm   10  avgt
5120.000 ±0.001B/op
ToString2.toString2Pass:·gc.alloc.rate.norm  100  avgt
5840.000 ±0.001B/op
ToString2.toString2Pass:·gc.alloc.rate.norm 1000  avgt
5   9848.004 ±0.001B/op

Don't know if it is something worth adding?

/Kasper

Re: [PATCH] Use StringJoiner where appropriate in java.base

2019-06-20 Thread Martin Buchholz 
On Wed, Jun 19, 2019 at 6:13 AM Сергей Цыпанов 
wrote:

>
> Also toString() of AtomicIntegerArray, AtomicLongArray and
> AtomicReferenceArray partially duplicate logic of corresponding method
> available in j.u.Arrays and can be replaced with call to delegate.
>

 This looks incorrect to me - it replaces volatile reads with plain reads,
breaking the happens-before guarantee, (but that's  hard to demonstrate in
a test).

Re: [PATCH] Use StringJoiner where appropriate in java.base

2019-06-20 Thread Kasper Nielsen 
On Thu, 20 Jun 2019 at 11:55, Peter Levart  wrote:
>
> Hi,
>
> On 6/20/19 10:50 AM, Kasper Nielsen wrote:
> > Hi,
> >
> > On Wed, 19 Jun 2019 at 14:12, Сергей Цыпанов  
> > wrote:
> >> Hello,
> >>
> >> in JDK code base we have many places (mainly in j.u.Arrays) where we 
> >> convert array to String using verbose constructions with StringBuilder.
> >>
> >> As far as we have got StringJoiner for a long time we can use it making 
> >> the code more simple.
> > It may make the code simpler, but it also comes with a hefty
> > performance penalty, taking twice as long in most cases compared to
> > the existing code.
> >
> > A quick benchmark toString'ing int arrays of size 1,10,100,1000
> >
> > Benchmark(size)  Mode  Cnt  Score Error  Units
> > ToString2.toStringExisting1  avgt5 16.675 ±   0.327  ns/op
> > ToString2.toStringExisting   10  avgt5 78.467 ±   1.373  ns/op
> > ToString2.toStringExisting  100  avgt5801.956 ±   7.517  ns/op
> > ToString2.toStringExisting 1000  avgt5  14944.235 ± 155.393  ns/op
> >
> > ToString2.toStringSuggested   1  avgt5 35.053 ±   0.533  ns/op
> > ToString2.toStringSuggested  10  avgt5222.043 ±  10.157  ns/op
> > ToString2.toStringSuggested 100  avgt5   2150.948 ±  13.285  ns/op
> > ToString2.toStringSuggested1000  avgt5  23411.264 ± 201.721  ns/op
> >
> > /Kasper
>
> StringJoiner may be faster if you already have String objects at hand,
> since it is able to exactly pre-size the target array and need not
> copy/resize it later, but if you append primitive types, then creating
> intermediate String objects referenced from a data structure - the
> StringJointer (so they can not be scalarized by JIT) is contra-productive.
>
> An interesting test would be to run Kasper's JMH benchmark with "-prof
> gc" option. I think it will show more garbage created per call too.

Yes, less garbage with StringBuilder (existing)

ToString2.toStringExisting:·gc.alloc.rate.norm 1  avgt
   5 80.000 ± 0.001B/op
ToString2.toStringExisting:·gc.alloc.rate.norm10  avgt
   5160.000 ± 0.001B/op
ToString2.toStringExisting:·gc.alloc.rate.norm   100  avgt
   5   1664.000 ± 0.001B/op
ToString2.toStringExisting:·gc.alloc.rate.norm  1000  avgt
   5  23536.006 ± 0.001B/op

ToString2.toStringSuggested:·gc.alloc.rate.norm1  avgt
   5168.000 ± 0.001B/op
ToString2.toStringSuggested:·gc.alloc.rate.norm   10  avgt
   5760.000 ± 0.001B/op
ToString2.toStringSuggested:·gc.alloc.rate.norm  100  avgt
   5   7144.001 ± 0.001B/op
ToString2.toStringSuggested:·gc.alloc.rate.norm 1000  avgt
   5  71024.010 ± 0.001B/op

If you allowed a two-time pass of the primitive array you could
actually create a version that only allocated the actual String and
backing array.
But don't know if it is worth it. But then again I'm always surprised
by the amount of string processing being done.

/Kasper

Re: [PATCH] Use StringJoiner where appropriate in java.base

2019-06-20 Thread Peter Levart 

Hi,

On 6/20/19 10:50 AM, Kasper Nielsen wrote:

Hi,

On Wed, 19 Jun 2019 at 14:12, Сергей Цыпанов  wrote:

Hello,

in JDK code base we have many places (mainly in j.u.Arrays) where we convert 
array to String using verbose constructions with StringBuilder.

As far as we have got StringJoiner for a long time we can use it making the 
code more simple.

It may make the code simpler, but it also comes with a hefty
performance penalty, taking twice as long in most cases compared to
the existing code.

A quick benchmark toString'ing int arrays of size 1,10,100,1000

Benchmark(size)  Mode  Cnt  Score Error  Units
ToString2.toStringExisting1  avgt5 16.675 ±   0.327  ns/op
ToString2.toStringExisting   10  avgt5 78.467 ±   1.373  ns/op
ToString2.toStringExisting  100  avgt5801.956 ±   7.517  ns/op
ToString2.toStringExisting 1000  avgt5  14944.235 ± 155.393  ns/op

ToString2.toStringSuggested   1  avgt5 35.053 ±   0.533  ns/op
ToString2.toStringSuggested  10  avgt5222.043 ±  10.157  ns/op
ToString2.toStringSuggested 100  avgt5   2150.948 ±  13.285  ns/op
ToString2.toStringSuggested1000  avgt5  23411.264 ± 201.721  ns/op

/Kasper


StringJoiner may be faster if you already have String objects at hand, 
since it is able to exactly pre-size the target array and need not 
copy/resize it later, but if you append primitive types, then creating 
intermediate String objects referenced from a data structure - the 
StringJointer (so they can not be scalarized by JIT) is contra-productive.


An interesting test would be to run Kasper's JMH benchmark with "-prof 
gc" option. I think it will show more garbage created per call too.


Regards, Peter

Re: [PATCH] Use StringJoiner where appropriate in java.base

2019-06-20 Thread Kasper Nielsen 
Hi,

On Wed, 19 Jun 2019 at 14:12, Сергей Цыпанов  wrote:
>
> Hello,
>
> in JDK code base we have many places (mainly in j.u.Arrays) where we convert 
> array to String using verbose constructions with StringBuilder.
>
> As far as we have got StringJoiner for a long time we can use it making the 
> code more simple.

It may make the code simpler, but it also comes with a hefty
performance penalty, taking twice as long in most cases compared to
the existing code.

A quick benchmark toString'ing int arrays of size 1,10,100,1000

Benchmark(size)  Mode  Cnt  Score Error  Units
ToString2.toStringExisting1  avgt5 16.675 ±   0.327  ns/op
ToString2.toStringExisting   10  avgt5 78.467 ±   1.373  ns/op
ToString2.toStringExisting  100  avgt5801.956 ±   7.517  ns/op
ToString2.toStringExisting 1000  avgt5  14944.235 ± 155.393  ns/op

ToString2.toStringSuggested   1  avgt5 35.053 ±   0.533  ns/op
ToString2.toStringSuggested  10  avgt5222.043 ±  10.157  ns/op
ToString2.toStringSuggested 100  avgt5   2150.948 ±  13.285  ns/op
ToString2.toStringSuggested1000  avgt5  23411.264 ± 201.721  ns/op

/Kasper

Re: [PATCH] Use StringJoiner where appropriate in java.base

2019-06-19 Thread Andrew Haley 
Hi,

On 6/19/19 2:12 PM, Сергей Цыпанов wrote:

> in JDK code base we have many places (mainly in j.u.Arrays) where we
> convert array to String using verbose constructions with
> StringBuilder.
> 
> As far as we have got StringJoiner for a long time we can use it
> making the code more simple.
> 
> Also toString() of AtomicIntegerArray, AtomicLongArray and
> AtomicReferenceArray partially duplicate logic of corresponding
> method available in j.u.Arrays and can be replaced with call to
> delegate.

I agree this makes the code apparently simpler, but it's not clear
that it's any faster, and there is always a cost in any large code
base associated with changes. This "churn", as it is called, has a
long-term cost for maintainers. Also, there is some risk in any part
of the core library that might be used when bootstrapping the system.

-- 
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. 
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

[PATCH] Use StringJoiner where appropriate in java.base

2019-06-19 Thread Сергей Цыпанов 
Hello,

in JDK code base we have many places (mainly in j.u.Arrays) where we convert 
array to String using verbose constructions with StringBuilder.

As far as we have got StringJoiner for a long time we can use it making the 
code more simple.

Also toString() of AtomicIntegerArray, AtomicLongArray and AtomicReferenceArray 
partially duplicate logic of corresponding method available in j.u.Arrays and 
can be replaced with call to delegate.

With kind regards,
Sergey Tsypanovdiff --git a/src/java.base/share/classes/java/util/Arrays.java b/src/java.base/share/classes/java/util/Arrays.java
--- a/src/java.base/share/classes/java/util/Arrays.java
+++ b/src/java.base/share/classes/java/util/Arrays.java
@@ -4879,18 +4879,14 @@
 public static String toString(long[] a) {
 if (a == null)
 return "null";
-int iMax = a.length - 1;
-if (iMax == -1)
+if (a.length == 0)
 return "[]";
 
-StringBuilder b = new StringBuilder();
-b.append('[');
-for (int i = 0; ; i++) {
-b.append(a[i]);
-if (i == iMax)
-return b.append(']').toString();
-b.append(", ");
+StringJoiner joiner = new StringJoiner(", ", "[", "]");
+for (long l : a) {
+joiner.add(Long.toString(l));
 }
+return joiner.toString();
 }
 
 /**
@@ -4909,18 +4905,14 @@
 public static String toString(int[] a) {
 if (a == null)
 return "null";
-int iMax = a.length - 1;
-if (iMax == -1)
+if (a.length == 0)
 return "[]";
 
-StringBuilder b = new StringBuilder();
-b.append('[');
-for (int i = 0; ; i++) {
-b.append(a[i]);
-if (i == iMax)
-return b.append(']').toString();
-b.append(", ");
+StringJoiner joiner = new StringJoiner(", ", "[", "]");
+for (int i : a) {
+joiner.add(Integer.toString(i));
 }
+return joiner.toString();
 }
 
 /**
@@ -4939,18 +4931,14 @@
 public static String toString(short[] a) {
 if (a == null)
 return "null";
-int iMax = a.length - 1;
-if (iMax == -1)
+if (a.length == 0)
 return "[]";
 
-StringBuilder b = new StringBuilder();
-b.append('[');
-for (int i = 0; ; i++) {
-b.append(a[i]);
-if (i == iMax)
-return b.append(']').toString();
-b.append(", ");
+StringJoiner joiner = new StringJoiner(", ", "[", "]");
+for (short s : a) {
+joiner.add(Short.toString(s));
 }
+return joiner.toString();
 }
 
 /**
@@ -4969,18 +4957,14 @@
 public static String toString(char[] a) {
 if (a == null)
 return "null";
-int iMax = a.length - 1;
-if (iMax == -1)
+if (a.length == 0)
 return "[]";
 
-StringBuilder b = new StringBuilder();
-b.append('[');
-for (int i = 0; ; i++) {
-b.append(a[i]);
-if (i == iMax)
-return b.append(']').toString();
-b.append(", ");
+StringJoiner joiner = new StringJoiner(", ", "[", "]");
+for (char c : a) {
+joiner.add(String.valueOf(c));
 }
+return joiner.toString();
 }
 
 /**
@@ -4999,18 +4983,14 @@
 public static String toString(byte[] a) {
 if (a == null)
 return "null";
-int iMax = a.length - 1;
-if (iMax == -1)
+if (a.length == 0)
 return "[]";
 
-StringBuilder b = new StringBuilder();
-b.append('[');
-for (int i = 0; ; i++) {
-b.append(a[i]);
-if (i == iMax)
-return b.append(']').toString();
-b.append(", ");
+StringJoiner joiner = new StringJoiner(", ", "[", "]");
+for (byte b : a) {
+joiner.add(Byte.toString(b));
 }
+return joiner.toString();
 }
 
 /**
@@ -5029,18 +5009,14 @@
 public static String toString(boolean[] a) {
 if (a == null)
 return "null";
-int iMax = a.length - 1;
-if (iMax == -1)
+if (a.length == 0)
 return "[]";
 
-StringBuilder b = new StringBuilder();
-b.append('[');
-for (int i = 0; ; i++) {
-b.append(a[i]);
-if (i == iMax)
-return b.append(']').toString();
-b.append(", ");
+StringJoiner joiner = new StringJoiner(", ", "[", "]");
+for (boolean b : a) {
+joiner.add(Boolean.toString(b));
 }
+return joiner.toString();
 }
 
 /**
@@ -5059,19 +5035,14 @@
 public static String toString(float[] a) {
 if (a == null)
 return "null";
-
-int iMax = a.length - 1;
-if (iMax == -1)
+if