From ff5762419580270d71812ab39fa1cc8f5c631dba Mon Sep 17 00:00:00 2001 From: Jesper Meyer Date: Fri, 7 Jul 2023 16:59:57 +0200 Subject: [PATCH] StringBuilder: use Span.Fill in Append repeating char (#86287) --- .../src/System/Text/StringBuilder.cs | 74 +++++++++++++------ .../tests/System/Text/StringBuilderTests.cs | 2 +- 2 files changed, 53 insertions(+), 23 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Text/StringBuilder.cs b/src/libraries/System.Private.CoreLib/src/System/Text/StringBuilder.cs index 6be755d595826..9a83624af1e92 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Text/StringBuilder.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Text/StringBuilder.cs @@ -659,36 +659,57 @@ public StringBuilder Append(char value, int repeatCount) return this; } - // this is where we can check if the repeatCount will put us over m_MaxCapacity - // We are doing the check here to prevent the corruption of the StringBuilder. - int newLength = Length + repeatCount; - if (newLength > m_MaxCapacity || newLength < repeatCount) + char[] chunkChars = m_ChunkChars; + int chunkLength = m_ChunkLength; + + // Try to fit the whole repeatCount in the current chunk + // Use the same check as Span.Slice for 64-bit so it can be folded + // Since repeatCount can't be negative, there's no risk for it to overflow on 32 bit + if (((nuint)(uint)chunkLength + (nuint)(uint)repeatCount) <= (nuint)(uint)chunkChars.Length) { - throw new ArgumentOutOfRangeException(nameof(repeatCount), SR.ArgumentOutOfRange_LengthGreaterThanCapacity); + chunkChars.AsSpan(chunkLength, repeatCount).Fill(value); + m_ChunkLength += repeatCount; } - - int index = m_ChunkLength; - while (repeatCount > 0) + else { - if (index < m_ChunkChars.Length) - { - m_ChunkChars[index++] = value; - --repeatCount; - } - else - { - m_ChunkLength = index; - ExpandByABlock(repeatCount); - Debug.Assert(m_ChunkLength == 0); - index = 0; - } + AppendWithExpansion(value, repeatCount); } - m_ChunkLength = index; AssertInvariants(); return this; } + private void AppendWithExpansion(char value, int repeatCount) + { + Debug.Assert(repeatCount > 0, "Invalid length; should have been validated by caller."); + + // Check if the repeatCount will put us over m_MaxCapacity + if ((uint)(repeatCount + Length) > (uint)m_MaxCapacity) + { + throw new ArgumentOutOfRangeException(nameof(repeatCount), SR.ArgumentOutOfRange_LengthGreaterThanCapacity); + } + + char[] chunkChars = m_ChunkChars; + int chunkLength = m_ChunkLength; + + // Fill the rest of the current chunk + int firstLength = chunkChars.Length - chunkLength; + if (firstLength > 0) + { + chunkChars.AsSpan(chunkLength, firstLength).Fill(value); + m_ChunkLength = chunkChars.Length; + } + + // Expand the builder to add another chunk + int restLength = repeatCount - firstLength; + ExpandByABlock(restLength); + Debug.Assert(m_ChunkLength == 0, "A new block was not created."); + + // Fill the new chunk with the remaining part of repeatCount + m_ChunkChars.AsSpan(0, restLength).Fill(value); + m_ChunkLength = restLength; + } + /// /// Appends a range of characters to the end of this builder. /// @@ -990,12 +1011,21 @@ public StringBuilder Append(char value) } else { - Append(value, 1); + AppendWithExpansion(value); } return this; } + [MethodImpl(MethodImplOptions.NoInlining)] + private void AppendWithExpansion(char value) + { + ExpandByABlock(1); + Debug.Assert(m_ChunkLength == 0, "A new block was not created."); + m_ChunkChars[0] = value; + m_ChunkLength++; + } + [CLSCompliant(false)] public StringBuilder Append(sbyte value) => AppendSpanFormattable(value); diff --git a/src/libraries/System.Runtime/tests/System/Text/StringBuilderTests.cs b/src/libraries/System.Runtime/tests/System/Text/StringBuilderTests.cs index cbe6cda0093f8..655ea981fb46d 100644 --- a/src/libraries/System.Runtime/tests/System/Text/StringBuilderTests.cs +++ b/src/libraries/System.Runtime/tests/System/Text/StringBuilderTests.cs @@ -550,7 +550,7 @@ public static void Append_Char_NoSpareCapacity_ThrowsArgumentOutOfRangeException var builder = new StringBuilder(0, 5); builder.Append("Hello"); - AssertExtensions.Throws("repeatCount", "requiredLength", () => builder.Append('a')); + AssertExtensions.Throws("requiredLength", () => builder.Append('a')); AssertExtensions.Throws("repeatCount", "requiredLength", () => builder.Append('a', 1)); }