From d39efa70a59c853b689a7bfc0afb5687920d8d25 Mon Sep 17 00:00:00 2001 From: James Ko Date: Sun, 21 Aug 2016 22:00:19 -0400 Subject: [PATCH] Further optimizations for String.Join, String.Concat (#6800) * Apply length-1 optimization to Join(string, object[]) * Apply length-1 optimization to Concat(IEnumerable) * Apply length-1 optimization to Concat(IEnumerable) * Apply lengths 0/1 optimizations to Concat(object[]), Concat(string[]) --- src/mscorlib/src/System/String.cs | 120 ++++++++++++++++++++++++++++++-------- 1 file changed, 95 insertions(+), 25 deletions(-) diff --git a/src/mscorlib/src/System/String.cs b/src/mscorlib/src/System/String.cs index df39181..4a51fb8 100644 --- a/src/mscorlib/src/System/String.cs +++ b/src/mscorlib/src/System/String.cs @@ -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 values) { if (values == null) @@ -3359,6 +3368,13 @@ namespace System { Contract.Ensures(Contract.Result() != 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(IEnumerable values) { + public static string Concat(IEnumerable values) + { if (values == null) throw new ArgumentNullException("values"); Contract.Ensures(Contract.Result() != null); Contract.EndContractBlock(); - StringBuilder result = StringBuilderCache.Acquire(); - using(IEnumerator en = values.GetEnumerator()) { - while (en.MoveNext()) { - T currentValue = en.Current; + using (IEnumerator 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 values) { + public static string Concat(IEnumerable values) + { if (values == null) throw new ArgumentNullException("values"); Contract.Ensures(Contract.Result() != null); Contract.EndContractBlock(); - StringBuilder result = StringBuilderCache.Acquire(); - using(IEnumerator en = values.GetEnumerator()) { - while (en.MoveNext()) { - result.Append(en.Current); - } + using (IEnumerator 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() != 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 -- 2.7.4