# HG changeset patch
# User igerasim
# Date 1407409653 -14400
# Node ID 0060a996fe1c0047030a09f5d0aa51c8a287388d
# Parent  71ee1f15845e3c5e293907992943f9c656fdcde0
8054221: StringJoiner imlementation optimization
Reviewed-by: martin

diff -r 71ee1f15845e -r 0060a996fe1c jdk/src/share/classes/java/util/StringJoiner.java
--- a/jdk/src/share/classes/java/util/StringJoiner.java	Thu Aug 07 13:04:26 2014 +0900
+++ b/jdk/src/share/classes/java/util/StringJoiner.java	Thu Aug 07 15:07:33 2014 +0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 2014, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -24,6 +24,9 @@
  */
 package java.util;
 
+import sun.misc.JavaLangAccess;
+import sun.misc.SharedSecrets;
+
 /**
  * {@code StringJoiner} is used to construct a sequence of characters separated
  * by a delimiter and optionally starting with a supplied prefix
@@ -67,22 +70,24 @@
     private final String delimiter;
     private final String suffix;
 
-    /*
-     * StringBuilder value -- at any time, the characters constructed from the
-     * prefix, the added element separated by the delimiter, but without the
-     * suffix, so that we can more easily add elements without having to jigger
-     * the suffix each time.
-     */
-    private StringBuilder value;
+    /** Contains all the string components added so far. */
+    private String[] elts;
+
+    /** The number of string components added so far. */
+    private int size;
 
-    /*
-     * By default, the string consisting of prefix+suffix, returned by
-     * toString(), or properties of value, when no elements have yet been added,
-     * i.e. when it is empty.  This may be overridden by the user to be some
-     * other value including the empty String.
+    /** Total length in chars so far, excluding prefix and suffix. */
+    private int len;
+
+    /**
+     * When overriden by the user to be non-null via {@link setEmptyValue}, the
+     * string returned by toString() when no elements have yet been added.
+     * When null, prefix + suffix is used as the empty value.
      */
     private String emptyValue;
 
+    private static final JavaLangAccess jla = SharedSecrets.getJavaLangAccess();
+
     /**
      * Constructs a {@code StringJoiner} with no characters in it, with no
      * {@code prefix} or {@code suffix}, and a copy of the supplied
@@ -125,7 +130,6 @@
         this.prefix = prefix.toString();
         this.delimiter = delimiter.toString();
         this.suffix = suffix.toString();
-        this.emptyValue = this.prefix + this.suffix;
     }
 
     /**
@@ -148,29 +152,44 @@
         return this;
     }
 
+    private static int getChars(String s, char[] chars, int start) {
+        int len = s.length();
+        s.getChars(0, len, chars, start);
+        return len;
+    }
+
     /**
      * Returns the current value, consisting of the {@code prefix}, the values
      * added so far separated by the {@code delimiter}, and the {@code suffix},
      * unless no elements have been added in which case, the
-     * {@code prefix + suffix} or the {@code emptyValue} characters are returned
+     * {@code prefix + suffix} or the {@code emptyValue} characters are returned.
      *
      * @return the string representation of this {@code StringJoiner}
      */
     @Override
     public String toString() {
-        if (value == null) {
+        final String[] elts = this.elts;
+        if (elts == null && emptyValue != null) {
             return emptyValue;
-        } else {
-            if (suffix.equals("")) {
-                return value.toString();
-            } else {
-                int initialLength = value.length();
-                String result = value.append(suffix).toString();
-                // reset value to pre-append initialLength
-                value.setLength(initialLength);
-                return result;
+        }
+        final int size = this.size;
+        final int addLen = prefix.length() + suffix.length();
+        if (addLen == 0) {
+            compactElts();
+            return size == 0 ? "" : elts[0];
+        }
+        final String delimiter = this.delimiter;
+        final char[] chars = new char[len + addLen];
+        int k = getChars(prefix, chars, 0);
+        if (size > 0) {
+            k += getChars(elts[0], chars, k);
+            for (int i = 1; i < size; i++) {
+                k += getChars(delimiter, chars, k);
+                k += getChars(elts[i], chars, k);
             }
         }
+        k += getChars(suffix, chars, k);
+        return jla.newStringUnsafe(chars);
     }
 
     /**
@@ -182,7 +201,16 @@
      * @return a reference to this {@code StringJoiner}
      */
     public StringJoiner add(CharSequence newElement) {
-        prepareBuilder().append(newElement);
+        final String elt = String.valueOf(newElement);
+        if (elts == null) {
+            elts = new String[8];
+        } else {
+            if (size == elts.length)
+                elts = Arrays.copyOf(elts, 2 * size);
+            len += delimiter.length();
+        }
+        len += elt.length();
+        elts[size++] = elt;
         return this;
     }
 
@@ -207,24 +235,25 @@
      */
     public StringJoiner merge(StringJoiner other) {
         Objects.requireNonNull(other);
-        if (other.value != null) {
-            final int length = other.value.length();
-            // lock the length so that we can seize the data to be appended
-            // before initiate copying to avoid interference, especially when
-            // merge 'this'
-            StringBuilder builder = prepareBuilder();
-            builder.append(other.value, other.prefix.length(), length);
+        if (other.elts == null) {
+            return this;
         }
-        return this;
+        other.compactElts();
+        return add(other.elts[0]);
     }
 
-    private StringBuilder prepareBuilder() {
-        if (value != null) {
-            value.append(delimiter);
-        } else {
-            value = new StringBuilder().append(prefix);
+    private void compactElts() {
+        if (size > 1) {
+            final char[] chars = new char[len];
+            int i = 1, k = getChars(elts[0], chars, 0);
+            do {
+                k += getChars(delimiter, chars, k);
+                k += getChars(elts[i], chars, k);
+                elts[i] = null;
+            } while (++i < size);
+            size = 1;
+            elts[0] = jla.newStringUnsafe(chars);
         }
-        return value;
     }
 
     /**
@@ -238,10 +267,7 @@
      * @return the length of the current value of {@code StringJoiner}
      */
     public int length() {
-        // Remember that we never actually append the suffix unless we return
-        // the full (present) value or some sub-string or length of it, so that
-        // we can add on more if we need to.
-        return (value != null ? value.length() + suffix.length() :
-                emptyValue.length());
+        return (size == 0 && emptyValue != null) ? emptyValue.length() :
+            len + prefix.length() + suffix.length();
     }
 }
diff -r 71ee1f15845e -r 0060a996fe1c jdk/test/java/util/StringJoiner/MergeTest.java
--- a/jdk/test/java/util/StringJoiner/MergeTest.java	Thu Aug 07 13:04:26 2014 +0900
+++ b/jdk/test/java/util/StringJoiner/MergeTest.java	Thu Aug 07 15:07:33 2014 +0400
@@ -23,7 +23,7 @@
 
 /**
  * @test
- * @bug 8017231 8020977
+ * @bug 8017231 8020977 8054221
  * @summary test  StringJoiner::merge
  * @run testng MergeTest
  */
@@ -36,98 +36,135 @@
 
 @Test
 public class MergeTest {
-    public void testNull() {
-        StringJoiner sj = new StringJoiner(",", "{", "}");
-        try {
-            sj.merge(null);
-            fail("Should throw NullPointerException!");
-        } catch (NullPointerException npe) {
-            // expected
+    private final static String[] PREFIXES = {"", "{", "@#$%"};
+    private final static String[] SUFFIXES = {"", "}", "*&%$"};
+
+    private static class Fixes {
+        public String pre0, suf0;
+        public String pre1, suf1;
+        public Fixes(String prefix0, String suffix0,
+                     String prefix1, String suffix1) {
+            this.pre0 = prefix0;
+            this.suf0 = suffix0;
+            this.pre1 = prefix1;
+            this.suf1 = suffix1;
         }
     }
 
-    public void testSimple() {
+    private static Stream<Fixes> fixesStream() {
+        Stream.Builder<Fixes> builder = Stream.builder();
+        for (final String prefix0 : PREFIXES) {
+            for (final String suffix0 : SUFFIXES) {
+                for (final String prefix1 : PREFIXES) {
+                    for (final String suffix1 : SUFFIXES) {
+                        builder.accept(new Fixes(prefix0, suffix0,
+                                                 prefix1, suffix1));
+                    }
+                }
+            }
+        }
+        return builder.build();
+    }
+
+    @Test(expectedExceptions = {NullPointerException.class})
+    public void testNull() {
         StringJoiner sj = new StringJoiner(",", "{", "}");
-        StringJoiner other = new StringJoiner(",", "[", "]");
-        Stream.of("a", "b", "c").forEachOrdered(sj::add);
-        Stream.of("d", "e", "f").forEachOrdered(other::add);
+        sj.merge(null);
+    }
 
-        sj.merge(other);
-        assertEquals(sj.toString(), "{a,b,c,d,e,f}");
+    public void testSimple() {
+        fixesStream().forEach(fixes -> {
+            StringJoiner sj = new StringJoiner(",", fixes.pre0, fixes.suf0);
+            StringJoiner other = new StringJoiner(",", fixes.pre1, fixes.suf1);
+            Stream.of("a", "b", "c").forEachOrdered(sj::add);
+            Stream.of("d", "e", "f").forEachOrdered(other::add);
+
+            sj.merge(other);
+            assertEquals(sj.toString(), fixes.pre0 + "a,b,c,d,e,f" + fixes.suf0);
+        });
     }
 
     public void testEmptyOther() {
-        StringJoiner sj = new StringJoiner(",", "{", "}");
-        StringJoiner other = new StringJoiner(",", "[", "]");
-        Stream.of("a", "b", "c").forEachOrdered(sj::add);
+        fixesStream().forEach(fixes -> {
+            StringJoiner sj = new StringJoiner(",", fixes.pre0, fixes.suf0);
+            StringJoiner other = new StringJoiner(",", fixes.pre1, fixes.suf1);
+            Stream.of("a", "b", "c").forEachOrdered(sj::add);
 
-        sj.merge(other);
-        assertEquals(sj.toString(), "{a,b,c}");
+            sj.merge(other);
+            assertEquals(sj.toString(), fixes.pre0 + "a,b,c" + fixes.suf0);
 
-        other.setEmptyValue("EMPTY");
-        sj.merge(other);
-        assertEquals(sj.toString(), "{a,b,c}");
+            other.setEmptyValue("EMPTY");
+            sj.merge(other);
+            assertEquals(sj.toString(), fixes.pre0 + "a,b,c" + fixes.suf0);
+        });
     }
 
     public void testEmptyThis() {
-        StringJoiner sj = new StringJoiner(",", "{", "}");
-        StringJoiner other = new StringJoiner(":", "[", "]");
-        Stream.of("d", "e", "f").forEachOrdered(other::add);
+        fixesStream().forEach(fixes -> {
+            StringJoiner sj = new StringJoiner(",", fixes.pre0, fixes.suf0);
+            StringJoiner other = new StringJoiner(":", fixes.pre1, fixes.suf1);
+            Stream.of("d", "e", "f").forEachOrdered(other::add);
 
-        sj.merge(other);
-        assertEquals(sj.toString(), "{d:e:f}");
+            sj.merge(other);
+            assertEquals(sj.toString(), fixes.pre0 + "d:e:f" + fixes.suf0);
 
-        sj = new StringJoiner(",", "{", "}").setEmptyValue("EMPTY");
-        assertEquals(sj.toString(), "EMPTY");
-        sj.merge(other);
-        assertEquals(sj.toString(), "{d:e:f}");
+            sj = new StringJoiner(",", fixes.pre0, fixes.suf0).setEmptyValue("EMPTY");
+            assertEquals(sj.toString(), "EMPTY");
+            sj.merge(other);
+            assertEquals(sj.toString(), fixes.pre0 + "d:e:f" + fixes.suf0);
+        });
     }
 
     public void testEmptyBoth() {
-        StringJoiner sj = new StringJoiner(",", "{", "}");
-        StringJoiner other = new StringJoiner(":", "[", "]");
+        fixesStream().forEach(fixes -> {
+            StringJoiner sj = new StringJoiner(",", fixes.pre0, fixes.suf0);
+            StringJoiner other = new StringJoiner(":", fixes.pre1, fixes.suf1);
 
-        sj.merge(other);
-        assertEquals(sj.toString(), "{}");
+            sj.merge(other);
+            assertEquals(sj.toString(), fixes.pre0 + fixes.suf0);
 
-        other.setEmptyValue("NOTHING");
-        sj.merge(other);
-        assertEquals(sj.toString(), "{}");
+            other.setEmptyValue("NOTHING");
+            sj.merge(other);
+            assertEquals(sj.toString(), fixes.pre0 + fixes.suf0);
 
-        sj = new StringJoiner(",", "{", "}").setEmptyValue("EMPTY");
-        assertEquals(sj.toString(), "EMPTY");
-        sj.merge(other);
-        assertEquals(sj.toString(), "EMPTY");
+            sj = new StringJoiner(",", fixes.pre0, fixes.suf0).setEmptyValue("EMPTY");
+            assertEquals(sj.toString(), "EMPTY");
+            sj.merge(other);
+            assertEquals(sj.toString(), "EMPTY");
+        });
     }
 
     public void testCascadeEmpty() {
-        StringJoiner sj = new StringJoiner(",", "{", "}");
-        StringJoiner o1 = new StringJoiner(":", "[", "]").setEmptyValue("Empty1");
-        StringJoiner o2 = new StringJoiner(",", "<", ">").setEmptyValue("Empty2");
+        fixesStream().forEach(fixes -> {
+            StringJoiner sj = new StringJoiner(",", fixes.pre0, fixes.suf0);
+            StringJoiner o1 = new StringJoiner(":", fixes.pre1, fixes.suf1).setEmptyValue("Empty1");
+            StringJoiner o2 = new StringJoiner(",", "<", ">").setEmptyValue("Empty2");
 
-        o1.merge(o2);
-        assertEquals(o1.toString(), "Empty1");
+            o1.merge(o2);
+            assertEquals(o1.toString(), "Empty1");
 
-        sj.merge(o1);
-        assertEquals(sj.toString(), "{}");
+            sj.merge(o1);
+            assertEquals(sj.toString(), fixes.pre0 + fixes.suf0);
+        });
     }
 
     public void testDelimiter() {
-        StringJoiner sj = new StringJoiner(",", "{", "}");
-        StringJoiner other = new StringJoiner(":", "[", "]");
-        Stream.of("a", "b", "c").forEachOrdered(sj::add);
-        Stream.of("d", "e", "f").forEachOrdered(other::add);
+        fixesStream().forEach(fixes -> {
+            StringJoiner sj = new StringJoiner(",", fixes.pre0, fixes.suf0);
+            StringJoiner other = new StringJoiner(":", fixes.pre1, fixes.suf1);
+            Stream.of("a", "b", "c").forEachOrdered(sj::add);
+            Stream.of("d", "e", "f").forEachOrdered(other::add);
 
-        sj.merge(other);
-        assertEquals(sj.toString(), "{a,b,c,d:e:f}");
+            sj.merge(other);
+            assertEquals(sj.toString(), fixes.pre0 + "a,b,c,d:e:f" + fixes.suf0);
+        });
     }
 
     public void testMergeSelf() {
-        final StringJoiner sj = new StringJoiner(",", "[", "]").add("a").add("b");
-        assertEquals(sj.merge(sj).toString(), "[a,b,a,b]");
-        assertEquals(sj.merge(sj).toString(), "[a,b,a,b,a,b,a,b]");
-
-        final StringJoiner sj2 = new StringJoiner(",").add("c").add("d");
-        assertEquals(sj2.merge(sj2).toString(), "c,d,c,d");
+        fixesStream().forEach(fixes -> {
+            final StringJoiner sj = new StringJoiner(",", fixes.pre0, fixes.suf0).add("a").add("b");
+            assertEquals(sj.merge(sj).toString(), fixes.pre0 + "a,b,a,b" + fixes.suf0);
+            assertEquals(sj.merge(sj).toString(), fixes.pre0 + "a,b,a,b,a,b,a,b" + fixes.suf0);
+        });
     }
 }
diff -r 71ee1f15845e -r 0060a996fe1c jdk/test/java/util/StringJoiner/StringJoinerTest.java
--- a/jdk/test/java/util/StringJoiner/StringJoinerTest.java	Thu Aug 07 13:04:26 2014 +0900
+++ b/jdk/test/java/util/StringJoiner/StringJoinerTest.java	Thu Aug 07 15:07:33 2014 +0400
@@ -305,9 +305,9 @@
         sj.add("2");
         assertEquals(sj.toString(), prefix + "1" + infix + "2" + suffix);
         sj.add("");
-        assertEquals(sj.toString(), prefix + "1" + infix + "2" +infix + suffix);
+        assertEquals(sj.toString(), prefix + "1" + infix + "2" + infix + suffix);
         sj.add("3");
-        assertEquals(sj.toString(), prefix + "1" + infix + "2" +infix + infix + "3" + suffix);
+        assertEquals(sj.toString(), prefix + "1" + infix + "2" + infix + infix + "3" + suffix);
     }
 
     public void testDelimiterCombinations() {