From: Stephen Toub Date: Mon, 22 May 2023 19:51:08 +0000 (-0400) Subject: Avoid ToString allocations for each ISpanFormattable value in string.Concat/Join... X-Git-Tag: accepted/tizen/unified/riscv/20231226.055536~2045 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=1df8a1959ac34f8b817a92cc35055ec5eb3cd76c;p=platform%2Fupstream%2Fdotnet%2Fruntime.git Avoid ToString allocations for each ISpanFormattable value in string.Concat/Join(..., IEnumerable) (#86521) We can use a T's ISpanFormattable implementation to avoid the individual ToStrings. We also don't need to maintain a separate implementation for Concat; at the cost of one branch per value, we can just reuse the Join implementation and pick up all of its optimizations for Concat. --- diff --git a/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs b/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs index 77251e0..df5a986 100644 --- a/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs +++ b/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs @@ -121,88 +121,8 @@ namespace System return result; } - public static string Concat(IEnumerable values) - { - ArgumentNullException.ThrowIfNull(values); - - if (typeof(T) == typeof(char)) - { - // Special-case T==char, as we can handle that case much more efficiently, - // and string.Concat(IEnumerable) can be used as an efficient - // enumerable-based equivalent of new string(char[]). - using (IEnumerator en = Unsafe.As>(values).GetEnumerator()) - { - if (!en.MoveNext()) - { - // There weren't any chars. Return the empty string. - return Empty; - } - - char c = en.Current; // save the first char - - if (!en.MoveNext()) - { - // There was only one char. Return a string from it directly. - return CreateFromChar(c); - } - - // Create the StringBuilder, add the chars we've already enumerated, - // add the rest, and then get the resulting string. - var result = new ValueStringBuilder(stackalloc char[StackallocCharBufferSizeLimit]); - result.Append(c); // first value - do - { - c = en.Current; - result.Append(c); - } - while (en.MoveNext()); - return result.ToString(); - } - } - else - { - 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; - } - - var result = new ValueStringBuilder(stackalloc char[StackallocCharBufferSizeLimit]); - - result.Append(firstString); - - do - { - currentValue = en.Current; - - if (currentValue != null) - { - result.Append(currentValue.ToString()); - } - } - while (en.MoveNext()); - - return result.ToString(); - } - } - } + public static string Concat(IEnumerable values) => + JoinCore(ReadOnlySpan.Empty, values); public static string Concat(IEnumerable values) { @@ -891,48 +811,102 @@ namespace System ThrowHelper.ThrowArgumentNullException(ExceptionArgument.values); } - using (IEnumerator en = values.GetEnumerator()) + using (IEnumerator e = values.GetEnumerator()) { - if (!en.MoveNext()) + if (!e.MoveNext()) { + // If the enumerator is empty, just return an empty string. return 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()) + if (typeof(T) == typeof(char)) { - // We have to handle the case of either currentValue - // or its ToString being null - return firstString ?? Empty; - } + // Special-case T==char, as we can handle that case much more efficiently, + // and string.Concat(IEnumerable) can be used as an efficient + // enumerable-based equivalent of new string(char[]). - var result = new ValueStringBuilder(stackalloc char[StackallocCharBufferSizeLimit]); + IEnumerator en = Unsafe.As>(e); - result.Append(firstString); + char c = en.Current; // save the first value + if (!en.MoveNext()) + { + // There was only one char. Return a string from it directly. + return CreateFromChar(c); + } - do + // Create the builder, add the char we already enumerated, + // add the rest, and then get the resulting string. + var result = new ValueStringBuilder(stackalloc char[StackallocCharBufferSizeLimit]); + result.Append(c); // first value + do + { + if (!separator.IsEmpty) + { + result.Append(separator); + } + + c = en.Current; + result.Append(c); + } + while (en.MoveNext()); + return result.ToString(); + } + else if (typeof(T).IsValueType && default(T) is ISpanFormattable) { - currentValue = en.Current; + // Special-case value types that are ISpanFormattable, as we can implement those to avoid + // all string allocations for the individual values. We only do this for value types because + // a) value types are much more likely to implement ISpanFormattable, and b) we can use + // DefaultInterpolatedStringHandler to do all the heavy lifting, and it's more efficient + // for value types as the checks it does for interface implementations are all elided. + + T value = e.Current; // save the first value + if (!e.MoveNext()) + { + // There was only one value. Return a string from it directly. + return value!.ToString() ?? Empty; + } - result.Append(separator); - if (currentValue != null) + var result = new DefaultInterpolatedStringHandler(0, 0, CultureInfo.CurrentCulture, stackalloc char[StackallocCharBufferSizeLimit]); + result.AppendFormatted(value); // first value + do { - result.Append(currentValue.ToString()); + if (!separator.IsEmpty) + { + result.AppendFormatted(separator); + } + + result.AppendFormatted(e.Current); } + while (e.MoveNext()); + return result.ToStringAndClear(); } - while (en.MoveNext()); + else + { + // For all other Ts, fall back to calling ToString on each and appending the resulting + // string to a builder. - return result.ToString(); + string? firstString = e.Current?.ToString(); // save the first value + if (!e.MoveNext()) + { + return firstString ?? Empty; + } + + var result = new ValueStringBuilder(stackalloc char[StackallocCharBufferSizeLimit]); + + result.Append(firstString); + do + { + if (!separator.IsEmpty) + { + result.Append(separator); + } + + result.Append(e.Current?.ToString()); + } + while (e.MoveNext()); + + return result.ToString(); + } } } @@ -965,6 +939,11 @@ namespace System } } + if (totalLength == 0) + { + return Empty; + } + // Copy each of the strings into the result buffer, interleaving with the separator. string result = FastAllocateString(totalLength); int copiedLength = 0;