Optimize _GrowableArray._join and _StringBase._interpolate.

runtime/lib/string_patch.dart

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 patch class String {
   /* patch */ factory String.fromCharCodes(Iterable<int> charCodes) {
     return _StringBase.createFromCharCodes(charCodes);
   }
 
   /* patch */ factory String.fromCharCode(int charCode) {
@@ skipping 512 matching lines @@
     final s = o.toString();
     if (s is! String) {
       throw new ArgumentError(o);
     }
     return s;
   }
 
   /**
    * Convert all objects in [values] to strings and concat them
    * into a result string.
+   * Modifies the input list if it contains non-`String` values.
    */
-  static String _interpolate(List<String> values) {
+  static String _interpolate(final List values) {
     final numValues = values.length;
-    _List stringList = new List<String>(numValues);
-    bool isOneByteString = true;
     int totalLength = 0;
     for (int i = 0; i < numValues; i++) {
-      var s = values[i].toString();
-      if (isOneByteString && (ClassID.getID(s) == ClassID.cidOneByteString)) {
+      final e = values[i];
+      final s = e.toString();
+      values[i] = s;

[Ivan Posva, 2014/09/16 14:57:32]

Again, non-properly looping loop. Please add comments as to what is going on and
ideally switch to a while statement.

[Lasse Reichstein Nielsen, 2014/09/18 09:30:37]

Comments added.
I've tried rewriting as while statement.
I', not sure it's more readable, but that might be author blindness.

The alternative I would normally use is:
  oneByteLoop: {
    for (...) {
       if (is not onebyteString) break oneByteLoop;
    }
    return _OneByteString._concatAll(...)
  }
  // handle non-onebyte case.

I am usually requested to rewrite that to something without labels, hence this
nested loop structure.

(Language feature request: for (...) {...} else { return ... } !)

+      if (ClassID.getID(s) == ClassID.cidOneByteString) {
         totalLength += s.length;
-      } else {
-        isOneByteString = false;
+        continue;
+      }
+      // Loops while the values convert to one-byte strings.
+      if (s is! String) {
+        throw new ArgumentError(s);
+      }
+      i++;
+      for (;i < numValues; i++) {
+        final e = values[i];
+        final s = e.toString();
+        values[i] = s;
         if (s is! String) {
           throw new ArgumentError(s);
         }
       }
-      stringList[i] = s;
+      return _concatRangeNative(values, 0, numValues);
     }
-    if (isOneByteString) {
-      return _OneByteString._concatAll(stringList, totalLength);
-    }
-    return _concatRangeNative(stringList, 0, stringList.length);
+    return _OneByteString._concatAll(values, totalLength);
   }
 
   Iterable<Match> allMatches(String string, [int start = 0]) {
     List<Match> result = new List<Match>();
     int length = string.length;
     int patternLength = this.length;
     int startIndex = start;
     while (true) {
       int position = string.indexOf(this, startIndex);
       if (position == -1) {
@@ skipping 97 matching lines @@
   int get hashCode native "String_getHashCode";
 
   bool _isWhitespace(int codeUnit) {
     return _StringBase._isOneByteWhitespace(codeUnit);
   }
 
   bool operator ==(Object other) {
     return super == other;
   }
 
+  String operator +(String other) {
+    if (ClassID.getID(other) != ClassID.cidOneByteString) {
+      return super + other;
+    }
+    final thisLength = this.length;
+    final otherLength = other.length;
+    final length = thisLength + otherLength;
+    if (length > 128) {
+      final list = new _List(2);
+      list[0] = this;
+      list[1] = other;
+      return _concatAll(list, length);
+    }
+    final _OneByteString result = _allocate(length);
+    for (int i = 0; i < thisLength; i++) {
+      result._setAt(i, this.codeUnitAt(i));
+    }
+    for (int i = 0; i < otherLength; i++) {
+      result._setAt(thisLength + i, other.codeUnitAt(i));
+    }
+    return result;
+  }
+
   String _substringUncheckedNative(int startIndex, int endIndex)
       native "OneByteString_substringUnchecked";
 
   List<String> _splitWithCharCode(int charCode)
       native "OneByteString_splitWithCharCode";
 
   List<String> split(Pattern pattern) {
     if ((ClassID.getID(pattern) == ClassID.cidOneByteString) &&
         (pattern.length == 1)) {
       return _splitWithCharCode(pattern.codeUnitAt(0));
     }
     return super.split(pattern);
   }
 
   // All element of 'strings' must be OneByteStrings.
   static _concatAll(List<String> strings, int totalLength) {
     // TODO(srdjan): Improve code below and raise or eliminate the limit.
     if (totalLength > 128) {
       // Native is quicker.
       return _StringBase._concatRangeNative(strings, 0, strings.length);
     }
-    var res = _OneByteString._allocate(totalLength);
+    final res = _OneByteString._allocate(totalLength);
     final stringsLength = strings.length;
     int rIx = 0;
     for (int i = 0; i < stringsLength; i++) {
-      _OneByteString e = strings[i];
+      final _OneByteString e = strings[i];
       final eLength = e.length;
       for (int s = 0; s < eLength; s++) {
         res._setAt(rIx++, e.codeUnitAt(s));
       }
     }
     return res;
   }
 
   int indexOf(Pattern pattern, [int start = 0]) {
     // Specialize for single character pattern.
@@ skipping 310 matching lines @@
 class _CodeUnits extends Object with ListMixin<int>,
                                      UnmodifiableListMixin<int> {
   /** The string that this is the code units of. */
   String _string;
 
   _CodeUnits(this._string);
 
   int get length => _string.length;
   int operator[](int i) => _string.codeUnitAt(i);
 }