StringBuilder: use Span.Fill in Append repeating char

[yesmey]

StringBuilder.Append(char value, int repeatCount) currently use a loop to add each character in sequence.
We can instead use Span<T>.Fill that is better optimized for this.

Note:
Append(char value) was calling this method as a fallback to when it needed to allocate. But with the new changes, Append(char value, int repeatCount) might be inlined, therefor I added a specific AppendWithExpansion(char) with MethodImplOptions.NoInlining.

Benchmark code
Benchmark result:

BenchmarkDotNet=v0.13.5, OS=Windows 11 (10.0.22621.1778/22H2/2022Update/SunValley2)
AMD Ryzen 7 3700X, 1 CPU, 16 logical and 8 physical cores
.NET SDK=8.0.100-preview.6.23276.3
[Host] : .NET 8.0.0 (8.0.23.27214), X64 RyuJIT AVX2
Job-WZCNRH : .NET 8.0.0 (42.42.42.42424), X64 RyuJIT AVX2
Job-RFXJAL : .NET 8.0.0 (42.42.42.42424), X64 RyuJIT AVX2

Method	Toolchain	amount	c	Mean	Ratio
AppendRepeated	main	1	a	2.169 ns	1.00
AppendRepeated	PR	1	a	2.623 ns	1.21
AppendRepeated	main	2	a	4.057 ns	1.00
AppendRepeated	PR	2	a	2.577 ns	0.64
AppendRepeated	main	4	a	7.857 ns	1.00
AppendRepeated	PR	4	a	3.035 ns	0.39
AppendRepeated	main	8	a	15.549 ns	1.00
AppendRepeated	PR	8	a	3.694 ns	0.24
AppendRepeated	main	16	a	31.002 ns	1.00
AppendRepeated	PR	16	a	3.979 ns	0.13

[ghost]

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

---

StringBuilder.Append(char value, int repeatCount) currently use a loop to add each character in sequence.
We can instead use Span<T>.Fill that is better optimized for this.

Note:
Append(char value) was calling this method as a fallback to when it needed to allocate. But with the new changes, Append(char value, int repeatCount) might be inlined, therefor I added a specific AppendWithExpansion(char) with MethodImplOptions.NoInlining.

Benchmark code
Benchmark result:

BenchmarkDotNet=v0.13.5, OS=Windows 11 (10.0.22621.1702/22H2/2022Update/SunValley2)
AMD Ryzen 7 3700X, 1 CPU, 16 logical and 8 physical cores
.NET SDK=8.0.100-preview.5.23255.2
[Host] : .NET 8.0.0 (8.0.23.25213), X64 RyuJIT AVX2
Job-FJHCLO : .NET 8.0.0 (42.42.42.42424), X64 RyuJIT AVX2
Job-DXTBEB : .NET 8.0.0 (42.42.42.42424), X64 RyuJIT AVX2

Method	Toolchain	amount	c	Mean	Ratio
AppendRepeatedConstAmount	main	?	?	8.393 ns	1.00
AppendRepeatedConstAmount	PR	?	?	3.210 ns	0.38
AppendRepeatedConst	main	1	?	3.488 ns	1.00
AppendRepeatedConst	PR	1	?	2.884 ns	0.83
AppendRepeatedConst	main	2	?	5.465 ns	1.00
AppendRepeatedConst	PR	2	?	2.733 ns	0.50
AppendRepeatedConst	main	4	?	8.640 ns	1.00
AppendRepeatedConst	PR	4	?	3.410 ns	0.39
AppendRepeatedConst	main	8	?	15.649 ns	1.00
AppendRepeatedConst	PR	8	?	3.849 ns	0.25
AppendRepeatedConst	main	16	?	30.945 ns	1.00
AppendRepeatedConst	PR	16	?	4.028 ns	0.13
AppendRepeated	main	1	a	3.491 ns	1.00
AppendRepeated	PR	1	a	4.069 ns	1.17
AppendRepeated	main	2	a	5.345 ns	1.00
AppendRepeated	PR	2	a	3.730 ns	0.70
AppendRepeated	main	4	a	8.662 ns	1.00
AppendRepeated	PR	4	a	4.304 ns	0.50
AppendRepeated	main	8	a	14.983 ns	1.00
AppendRepeated	PR	8	a	4.807 ns	0.32
AppendRepeated	main	16	a	29.593 ns	1.00
AppendRepeated	PR	16	a	5.170 ns	0.17

Author:	yesmey
Assignees:	-
Labels:	area-System.Runtime, community-contribution
Milestone:	-

[IDisposable]

Insert

Debug.Assert(firstLength<repeatCount,"We shouldn't be called if there was enough space for the entire run.");

[adamsitnik]

LGTM, thank you @yesmey not just for the contribution but also for providing benchmark numbers and their source code!

Would you be interested in contributing a benchmark for Append(char, int) to https://github.com/dotnet/performance/blob/main/src/benchmarks/micro/libraries/System.Text/Perf.StringBuilder.cs? Currently we have no benchmarks for it and our reporting system won't report any improvements once I merge your PR.

[yesmey]

@adamsitnik The regression in #88673 comes from marking the allocating path of Append(char) with MethodImplOptions.NoInlining. That benchmark in particular is allocating fairly often, and it makes sense the slow path is faster when it previously was being inlined more aggressively. This pattern is fairly common across the runtime, but if you prefer I can remove it and see if its better?

[adamsitnik]

I can remove it and see if its better?

@yesmey could you please give it a try?

I was also thinking about doing sth like this:

publicStringBuilderAppend(charvalue){intnextCharIndex=m_ChunkLength;char[]chars=m_ChunkChars;if((uint)chars.Length==(uint)nextCharIndex){ExpandByABlock(1);nextCharIndex=0;chars=m_ChunkChars;}chars[nextCharIndex]=value;m_ChunkLength++;returnthis;}

but I am not sure if array boundaries check would got removed.

[MichalPetryka]

but I am not sure if array boundaries check would got removed.

You can avoid them like this:

Unsafe.Add(refMemoryMarshal.GetArrayDataReference(chars),(uint)nextCharIndex)= value;

[yesmey]

Sorry about the delay. I'm gonna need some help on this one.

Having trouble reproducing this locally. I think its because most of the benchmark is spent allocating and the Error column is sometimes very high. I tried to turn off as much as possible on my machine while running the benchmarks.

but I am not sure if array boundaries check would got removed.

You can avoid them like this:

Unsafe.Add(refMemoryMarshal.GetArrayDataReference(chars),(uint)nextCharIndex)= value;

I doubt avoiding an array boundaries check would even be noticeable here since the allocation size doubles every time, so the times that code is executed is gonna be rare, and sacrificing bounds safety for that is likely not worth it

My hypothesis is that the slowdown is coming from the benchmark calling this very frequently in a loop, and the jit is able to inline so the read/write of m_ChunkLength better in the previous version.

Here's how I build and run the benchmark locally

.\build.cmd Clr+Clr.Aot+Libs -c Release -rc Release

and

python3 .\scripts\benchmarks_ci.py -f net8.0 --filter System.Text.Tests.Perf_StringBuilder.Append_Char --corerun "D:\dotnet\runtime\artifacts\bin\testhost\net8.0-windows-Release-x64\shared\Microsoft.NETCore.App\8.0.0\CoreRun.exe""D:\dotnet\yesmey_runtime\artifacts\bin\testhost\net8.0-windows-Release-x64\shared\Microsoft.NETCore.App\8.0.0\CoreRun.exe" --bdn-artifacts C:\results\after_test

\yesmey_runtime...CoreRun.exe built from ff57624
\runtime...CoreRun.exe built from 5a03596 (parent)

BenchmarkDotNet=v0.13.2.2052-nightly, OS=Windows 11 (10.0.22621.1992)
AMD Ryzen 7 7800X3D, 1 CPU, 16 logical and 8 physical cores
.NET SDK=8.0.100-preview.7.23364.32
[Host] : .NET 8.0.0 (8.0.23.36403), X64 RyuJIT AVX2
Job-DWSUBF : .NET 8.0.0 (42.42.42.42424), X64 RyuJIT AVX2
Job-YNMWPS : .NET 8.0.0 (42.42.42.42424), X64 RyuJIT AVX2
PowerPlanMode=00000000-0000-0000-0000-000000000000 Arguments=/p:EnableUnsafeBinaryFormatterSerialization=true IterationTime=250.0000 ms MaxIterationCount=20 MinIterationCount=15 WarmupCount=1 

Method	Job	Toolchain	length	Mean	Error	StdDev	Median	Min	Max	Ratio	Gen0	Gen1	Allocated	Alloc Ratio
Append_Char	Job-DWSUBF	\runtime\artifacts\bin\testhost\net8.0-windows-Release-x64\shared\Microsoft.NETCore.App\8.0.0\CoreRun.exe	100	90.69 ns	0.659 ns	0.584 ns	90.50 ns	89.93 ns	91.68 ns	1.00	0.0108	-	544 B	1.00
Append_Char	Job-YNMWPS	\yesmey_runtime\artifacts\bin\testhost\net8.0-windows-Release-x64\shared\Microsoft.NETCore.App\8.0.0\CoreRun.exe	100	97.78 ns	0.240 ns	0.201 ns	97.72 ns	97.61 ns	98.18 ns	1.08	0.0106	-	544 B	1.00
Append_Char	Job-DWSUBF	\runtime\artifacts\bin\testhost\net8.0-windows-Release-x64\shared\Microsoft.NETCore.App\8.0.0\CoreRun.exe	100000	51,838.73 ns	170.468 ns	142.348 ns	51,837.67 ns	51,624.53 ns	52,088.30 ns	1.00	4.1254	3.5066	209968 B	1.00
Append_Char	Job-YNMWPS	\yesmey_runtime\artifacts\bin\testhost\net8.0-windows-Release-x64\shared\Microsoft.NETCore.App\8.0.0\CoreRun.exe	100000	51,924.36 ns	184.830 ns	163.847 ns	51,939.05 ns	51,721.98 ns	52,278.99 ns	1.00	4.1391	3.5182	209968 B	1.00

Here's the asm output:

Before

; System.Text.Tests.Perf_StringBuilder.Append_Char(Int32)pushrdipushrsipushrbppushrbxsubrsp,28movebx,edxmovrcx,offset MT_System.Text.StringBuildercall CORINFO_HELP_NEWSFASTmovrsi,raxmov dword ptr [rsi+20],7FFFFFFFmovrcx,offset MT_System.Char[]movedx,10call CORINFO_HELP_NEWARR_1_VClearcx,[rsi+8]movrdx,raxcall CORINFO_HELP_ASSIGN_REFxoredi,editestebx,ebxjle short M00_L02M00_L00:movecx,[rsi+18]movedx,ecxmovrax,[rsi+8]movr8d,[rax+8]cmpr8d,edxjbe short M00_L03movedx,edxmov word ptr [rax+rdx*2+10],61incecxmov[rsi+18],ecxM00_L01:incedicmpedi,ebxjl short M00_L00M00_L02:movrax,rsiaddrsp,28poprbxpoprbppoprsipoprdiretM00_L03:movebp,1movedx,[rsi+1C]leaedx,[rdx+rcx+1]cmpedx,[rsi+20]jg short M00_L07testedx,edxjle short M00_L07M00_L04:movrdx,[rsi+8]cmp[rdx+8],ecxjle short M00_L05leaeax,[rcx+1]cmpecx,[rdx+8]jae short M00_L08movecx,ecxmov word ptr [rdx+rcx*2+10],61decebpjmp short M00_L06M00_L05:mov[rsi+18],ecxmovrcx,rsimovedx,ebpcall qword ptr [7FFE1853D518]; System.Text.StringBuilder.ExpandByABlock(Int32)xorecx,ecxmoveax,ecxM00_L06:testebp,ebpmovecx,eaxjg short M00_L04mov[rsi+18],ecxjmp short M00_L01M00_L07:movrcx,offset MT_System.ArgumentOutOfRangeExceptioncall CORINFO_HELP_NEWSFASTmovrbx,raxmovecx,18331movrdx,7FFE180A4000call CORINFO_HELP_STRCNSmovrsi,raxcall qword ptr [7FFE18786790]movr8,raxmovrdx,rsimovrcx,rbxcall qword ptr [7FFE182B6F28]movrcx,rbxcall CORINFO_HELP_THROWint3M00_L08:call CORINFO_HELP_RNGCHKFAILint3; Total bytes of code 280

After

; System.Text.Tests.Perf_StringBuilder.Append_Char(Int32)pushrdipushrsipushrbxsubrsp,20movebx,edxmovrcx,offset MT_System.Text.StringBuildercall CORINFO_HELP_NEWSFASTmovrsi,raxmov dword ptr [rsi+20],7FFFFFFFmovrcx,offset MT_System.Char[]movedx,10call CORINFO_HELP_NEWARR_1_VClearcx,[rsi+8]movrdx,raxcall CORINFO_HELP_ASSIGN_REFxoredi,editestebx,ebxjle short M00_L02M00_L00:movecx,[rsi+18]movedx,ecxmovrax,[rsi+8]movr8d,[rax+8]cmpr8d,edxjbe short M00_L03movedx,edxmov word ptr [rax+rdx*2+10],61incecxmov[rsi+18],ecxM00_L01:incedicmpedi,ebxjl short M00_L00M00_L02:movrax,rsiaddrsp,20poprbxpoprsipoprdiretM00_L03:movrcx,rsimovedx,61call qword ptr [7FFE1853C3F0]; System.Text.StringBuilder.AppendWithExpansion(Char)jmp short M00_L01; Total bytes of code 137

; System.Text.StringBuilder.AppendWithExpansion(Char)pushrsipushrbxsubrsp,28movrbx,rcxmovesi,edxmovrcx,rbxmovedx,1call qword ptr [7FFE1853CB58]; System.Text.StringBuilder.ExpandByABlock(Int32)movrax,[rbx+8]cmp dword ptr [rax+8],0jbe short M01_L00mov[rax+10],siinc dword ptr [rbx+18]addrsp,28poprbxpoprsiretM01_L00:call CORINFO_HELP_RNGCHKFAILint3; Total bytes of code 55