Further optimizations for String.Join, String.Concat (#6800)
authorJames Ko <jamesqko@gmail.com>
Mon, 22 Aug 2016 02:00:19 +0000 (22:00 -0400)
committerJan Kotas <jkotas@microsoft.com>
Mon, 22 Aug 2016 02:00:19 +0000 (19:00 -0700)
* Apply length-1 optimization to Join(string, object[])
* Apply length-1 optimization to Concat(IEnumerable<string>)
* Apply length-1 optimization to Concat(IEnumerable<T>)
* Apply lengths 0/1 optimizations to Concat(object[]), Concat(string[])

src/mscorlib/src/System/String.cs

index df39181..4a51fb8 100644 (file)
@@ -78,24 +78,35 @@ namespace System {
         }
 
         [ComVisible(false)]
-        public static String Join(String separator, params Object[] values) {
-            if (values==null)
+        public static string Join(string separator, params object[] values)
+        {
+            if (values == null)
                 throw new ArgumentNullException("values");
             Contract.EndContractBlock();
 
             if (values.Length == 0 || values[0] == null)
-                return String.Empty;
+                return string.Empty;
 
-            StringBuilder result = StringBuilderCache.Acquire();
+            string firstString = values[0].ToString();
 
-            result.Append(values[0].ToString());
+            if (values.Length == 1)
+            {
+                return firstString ?? string.Empty;
+            }
+
+            StringBuilder result = StringBuilderCache.Acquire();
+            result.Append(firstString);
 
-            for (int i = 1; i < values.Length; i++) {
+            for (int i = 1; i < values.Length; i++)
+            {
                 result.Append(separator);
-                if (values[i] != null) {
-                    result.Append(values[i].ToString());
+                object value = values[i];
+                if (value != null)
+                {
+                    result.Append(value.ToString());
                 }
             }
+
             return StringBuilderCache.GetStringAndRelease(result);
         }
 
@@ -150,8 +161,6 @@ namespace System {
             }
         }
 
-
-
         [ComVisible(false)]
         public static String Join(String separator, IEnumerable<String> values) {
             if (values == null)
@@ -3359,6 +3368,13 @@ namespace System {
             Contract.Ensures(Contract.Result<String>() != null);
             Contract.EndContractBlock();
 
+            if (args.Length <= 1)
+            {
+                return args.Length == 0 ?
+                    string.Empty :
+                    args[0]?.ToString() ?? string.Empty;
+            }
+
             // We need to get an intermediary string array
             // to fill with each of the args' ToString(),
             // and then just concat that in one operation.
@@ -3411,40 +3427,87 @@ namespace System {
         }
 
         [ComVisible(false)]
-        public static String Concat<T>(IEnumerable<T> values) {
+        public static string Concat<T>(IEnumerable<T> values)
+        {
             if (values == null)
                 throw new ArgumentNullException("values");
             Contract.Ensures(Contract.Result<String>() != null);
             Contract.EndContractBlock();
 
-            StringBuilder result = StringBuilderCache.Acquire();
-            using(IEnumerator<T> en = values.GetEnumerator()) {
-                while (en.MoveNext()) {
-                    T currentValue = en.Current;
+            using (IEnumerator<T> en = values.GetEnumerator())
+            {
+                if (!en.MoveNext())
+                    return string.Empty;
+                
+                // We called MoveNext once, so this will be the first item
+                T currentValue = en.Current;
+
+                // Call ToString before calling MoveNext again, since
+                // we want to stay consistent with the below loop
+                // Everything should be called in the order
+                // MoveNext-Current-ToString, unless further optimizations
+                // can be made, to avoid breaking changes
+                string firstString = currentValue?.ToString();
+
+                // If there's only 1 item, simply call ToString on that
+                if (!en.MoveNext())
+                {
+                    // We have to handle the case of either currentValue
+                    // or its ToString being null
+                    return firstString ?? string.Empty;
+                }
+
+                StringBuilder result = StringBuilderCache.Acquire();
+                
+                result.Append(firstString);
 
-                    if (currentValue != null) {
+                do
+                {
+                    currentValue = en.Current;
+
+                    if (currentValue != null)
+                    {
                         result.Append(currentValue.ToString());
                     }
-                }            
+                }
+                while (en.MoveNext());
+
+                return StringBuilderCache.GetStringAndRelease(result);
             }
-            return StringBuilderCache.GetStringAndRelease(result);
         }
 
 
         [ComVisible(false)]
-        public static String Concat(IEnumerable<String> values) {
+        public static string Concat(IEnumerable<string> values)
+        {
             if (values == null)
                 throw new ArgumentNullException("values");
             Contract.Ensures(Contract.Result<String>() != null);
             Contract.EndContractBlock();
 
-            StringBuilder result = StringBuilderCache.Acquire();
-            using(IEnumerator<String> en = values.GetEnumerator()) {
-                while (en.MoveNext()) {
-                     result.Append(en.Current);
-                }            
+            using (IEnumerator<string> en = values.GetEnumerator())
+            {
+                if (!en.MoveNext())
+                    return string.Empty;
+                
+                string firstValue = en.Current;
+
+                if (!en.MoveNext())
+                {
+                    return firstValue ?? string.Empty;
+                }
+
+                StringBuilder result = StringBuilderCache.Acquire();
+                result.Append(firstValue);
+
+                do
+                {
+                    result.Append(en.Current);
+                }
+                while (en.MoveNext());
+
+                return StringBuilderCache.GetStringAndRelease(result);
             }
-            return StringBuilderCache.GetStringAndRelease(result);            
         }
 
 
@@ -3559,6 +3622,13 @@ namespace System {
             Contract.Ensures(Contract.Result<String>() != null);
             Contract.EndContractBlock();
 
+            if (values.Length <= 1)
+            {
+                return values.Length == 0 ?
+                    string.Empty :
+                    values[0] ?? string.Empty;
+            }
+
             // It's possible that the input values array could be changed concurrently on another
             // thread, such that we can't trust that each read of values[i] will be equivalent.
             // Worst case, we can make a defensive copy of the array and use that, but we first