Uh oh!
There was an error while loading. Please reload this page.
Improve String.Substring performance - #62577
Conversation
ghost
commented
Dec 9, 2021
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
nietras
commented
Dec 9, 2021
Using Span.Slice guard clauses for perf. Regressions I believe are due to benchmark issues. In any case, is changing guard clauses to this an acceptable change? Should exception messages be preserved? BenchmarkDotNet=v0.13.1.1620-nightly, OS=Windows 10.0.19044.1348 (21H2)
AMD Ryzen 9 5950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK=7.0.100-alpha.1.21568.2
[Host] : .NET 6.0.0 (6.0.21.48005), X64 RyuJIT
Job-ICQHYC : .NET 7.0.0 (42.42.42.42424), X64 RyuJIT
Job-KPKVVD : .NET 7.0.0 (42.42.42.42424), X64 RyuJIT
PowerPlanMode=00000000-0000-0000-0000-000000000000 Arguments=/p:DebugType=portable,-bl:benchmarkdotnet.binlog IterationTime=250.0000 ms MaxIterationCount=20 MinIterationCount=15 WarmupCount=1
|
ghost
commented
Dec 9, 2021
Tagging subscribers to this area: @dotnet/area-system-runtime Issue DetailsWIP just to get the ball rolling regarding discussion of changing exception messages or similar. cc @adamsitnik@GrabYourPitchforks@stephentoub as discussed in #60463 Change to BenchmarkDotNet=v0.13.1.1620-nightly, OS=Windows 10.0.19044.1348 (21H2)
AMD Ryzen 9 5950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK=7.0.100-alpha.1.21568.2
[Host] : .NET 6.0.0 (6.0.21.48005), X64 RyuJIT
Job-FWUOEW : .NET 7.0.0 (42.42.42.42424), X64 RyuJIT
Job-HPGSVV : .NET 7.0.0 (42.42.42.42424), X64 RyuJIT
PowerPlanMode=00000000-0000-0000-0000-000000000000 Arguments=/p:DebugType=portable,-bl:benchmarkdotnet.binlog IterationTime=250.0000 ms MaxIterationCount=20 MinIterationCount=15 WarmupCount=1
|
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
nietras
commented
Dec 10, 2021
@GrabYourPitchforks tried to address feedback. Still don't know what to do about getting reliable benchmark tests. These do not show expected results currently but are also volatile. Substring(0) still faster of course. Tried running Let me know, what next steps you guys think should be. |
Uh oh!
There was an error while loading. Please reload this page.
nietras
commented
Dec 14, 2021
Cleaned up. Benchmarks below, only significant improvements for when returning same string or empty. Regressions are often due to variance in benchmarks. Main point probably is throw code is separated out reducing code size of the method. I have included the cost of new'ing a similar string for comparison here. Substring(int)
Substring(int,int)
ASMM; System.Tests.Perf_String_Substring_IntInt.Substring()subrsp,28movr8,[rcx+8]movrcx,1B31ACA8048movrcx,[rcx]movedx,[r8+8]movr8d,[r8+0C]call System.String.Substring(Int32, Int32)nopaddrsp,28ret; Total bytes of code 40; System.String.Substring(Int32, Int32)pushrdipushrsipushrbxsubrsp,20movrdi,rcxmovesi,edxtestesi,esijl near ptr M01_L03movebx,[rdi+8]cmpebx,esijl near ptr M01_L04testr8d,r8djl near ptr M01_L05movecx,ebxsubecx,r8dcmpecx,esijl near ptr M01_L06testr8d,r8dje short M01_L02testesi,esije short M01_L01M01_L00:movecx,r8dcall System.String.FastAllocateString(Int32)movrbx,raxmovr8d,[rbx+8]learcx,[rbx+0C]addrdi,0Cmovedx,esileardx,[rdi+rdx*2]addr8,r8call System.Buffer.Memmove(Byte ByRef, Byte ByRef, UIntPtr)movrax,rbxaddrsp,20poprbxpoprsipoprdiretM01_L01:cmpebx,r8djne short M01_L00movrax,rdiaddrsp,20poprbxpoprsipoprdiretM01_L02:movrax,1B31ACA3020movrax,[rax]addrsp,20poprbxpoprsipoprdiretM01_L03:movrcx,offset MT_System.ArgumentOutOfRangeExceptioncall CORINFO_HELP_NEWSFASTmovrsi,raxmovecx,19A4movrdx,7FF7DA114000call CORINFO_HELP_STRCNSmovrdi,raxmovecx,0A855movrdx,7FF7DA114000call CORINFO_HELP_STRCNSmovrcx,raxcall System.SR.GetResourceString(System.String)movr8,raxmovrdx,rdimovrcx,rsicall System.ArgumentOutOfRangeException..ctor(System.String, System.String)movrcx,rsicall CORINFO_HELP_THROWM01_L04:movrcx,offset MT_System.ArgumentOutOfRangeExceptioncall CORINFO_HELP_NEWSFASTmovrsi,raxmovecx,19A4movrdx,7FF7DA114000call CORINFO_HELP_STRCNSmovrdi,raxmovecx,0A891movrdx,7FF7DA114000call CORINFO_HELP_STRCNSmovrcx,raxcall System.SR.GetResourceString(System.String)movr8,raxmovrdx,rdimovrcx,rsicall System.ArgumentOutOfRangeException..ctor(System.String, System.String)movrcx,rsicall CORINFO_HELP_THROWM01_L05:movrcx,offset MT_System.ArgumentOutOfRangeExceptioncall CORINFO_HELP_NEWSFASTmovrsi,raxmovecx,57movrdx,7FF7DA114000call CORINFO_HELP_STRCNSmovrdi,raxmovecx,0A519movrdx,7FF7DA114000call CORINFO_HELP_STRCNSmovrcx,raxcall System.SR.GetResourceString(System.String)movr8,raxmovrdx,rdimovrcx,rsicall System.ArgumentOutOfRangeException..ctor(System.String, System.String)movrcx,rsicall CORINFO_HELP_THROWM01_L06:movrcx,offset MT_System.ArgumentOutOfRangeExceptioncall CORINFO_HELP_NEWSFASTmovrsi,raxmovecx,57movrdx,7FF7DA114000call CORINFO_HELP_STRCNSmovrdi,raxmovecx,9EBDmovrdx,7FF7DA114000call CORINFO_HELP_STRCNSmovrcx,raxcall System.SR.GetResourceString(System.String)movr8,raxmovrdx,rdimovrcx,rsicall System.ArgumentOutOfRangeException..ctor(System.String, System.String)movrcx,rsicall CORINFO_HELP_THROWint3; Total bytes of code 512PR; System.Tests.Perf_String_Substring_IntInt.Substring()subrsp,28movr8,[rcx+8]movrcx,1FA62268048movrcx,[rcx]movedx,[r8+8]movr8d,[r8+0C]call System.String.Substring(Int32, Int32)nopaddrsp,28ret; Total bytes of code 40; System.String.Substring(Int32, Int32)pushrdipushrsipushrbxsubrsp,20movrsi,rcxmovedi,edxmovecx,[rsi+8]moveax,edimovedx,r8daddrax,rdxmovedx,ecxcmprax,rdxja short M01_L02testr8d,r8djne short M01_L00movrax,1FA62263020movrax,[rax]addrsp,20poprbxpoprsipoprdiretM01_L00:cmpr8d,ecxjne short M01_L01movrax,rsiaddrsp,20poprbxpoprsipoprdiretM01_L01:movecx,r8dcall System.String.FastAllocateString(Int32)movrbx,raxmovr8d,[rbx+8]learcx,[rbx+0C]addrsi,0Cmovedx,edileardx,[rsi+rdx*2]addr8,r8call System.Buffer.Memmove(Byte ByRef, Byte ByRef, UIntPtr)movrax,rbxaddrsp,20poprbxpoprsipoprdiretM01_L02:movrcx,rsimovedx,edicall System.String.ThrowSubstringArgumentOutRangeException(Int32, Int32)int3; Total bytes of code 131 |
nietras
commented
Jan 25, 2022
@GrabYourPitchforks should I close this or? |
danmoseley
commented
May 2, 2022
I'm going to close this as the improvements are as you say marginal. Thank you for the attempt! |
WIP just to get the ball rolling regarding discussion of changing exception messages or similar. cc @adamsitnik@GrabYourPitchforks@stephentoub as discussed in #60463
Change to
Substring(int startIndex)improvements (note highly volatile). This comes with all the caveats of potential increases code size etc.