Creating StringBuilder with large initial capacity has regressed #59142
Comments
adamsitnik
added
area-System.Runtime
tenet-performance
|
Tagging subscribers to this area: @dotnet/area-system-runtime Issue DetailsRepro: git clone https://github.com/dotnet/performance.git
py .\performance\scripts\benchmarks_ci.py -f net5.0 net6.0 --filter "System.Text.Tests.Perf_StringBuilder.ctor_capacity"It's not specific to any OS, it repros on both Windows and Unix:
@DrewScoggins it seems that it got detected by the bot in dotnet/perf-autofiling-issues#304, but it got reported a month after it had actually happened. Do we know why?
System.Text.Tests.Perf_StringBuilder.ctor_capacity(length: 100000)
|
dotnet-issue-labeler
bot
added
the
untriaged
|
Some other benchmarks that use large string builders like |
This was because we did a rerun of the bot over that time period, after we did lab wide reruns over that time period because of missing data. |
jeffhandley
added
needs-further-triage
|
Ok, so I investigated the regression. Here is what I've found: All this benchmark does, is creating a Which is more or less exercising runtime/src/libraries/System.Private.CoreLib/src/System/Text/StringBuilder.cs Lines 180 to 181 in cf49643
which according to git blame was not modified for at least 2 years which does not answer the question why it has regressed. By using the following command: git clone https://github.com/dotnet/performance.git
py .\performance\scripts\benchmarks_ci.py -f net5.0 net6.0 --filter *ctor_capacity --bdn-arguments "--invocationCount 75088 --iterationCount 20 --profiler ETW"I was able to capture two ETW trace files, filter them to a single benchmark iteration that performed exactly the same work (see
After loading all the symbols, disabling folding, filtering out Finalizer and Tiered JIT threads I got the following output for .NET 5:
and .NET 6:
and compared them using PerfView: Overweight report for symbols common between both files
In this report, overweight is ratio of actual growth compared to 121.9%. Interest level attempts to identify smaller methods which changed a lot. These are likely the most interesting frames to sart investigating An overweight of greater than 100% indicates the symbol grew in cost more than average. High overweights are a likely source of regressions and represent good places to investigate. Only symbols that have at least 2% impact are shown.
It seems that garbage collection of large arrays became more expensive compared to .NET 5. @Maoni0 is it something expected and we should just close the issue? I'am moving this issue to 7.0 and to GC area as I did all I could on my side. FWIW my env info: OS=Windows 10.0.19043.1288 (21H1/May2021Update) |
|
Tagging subscribers to this area: @dotnet/gc Issue DetailsRepro: git clone https://github.com/dotnet/performance.git
py .\performance\scripts\benchmarks_ci.py -f net5.0 net6.0 --filter "System.Text.Tests.Perf_StringBuilder.ctor_capacity"It's not specific to any OS, it repros on both Windows and Unix:
@DrewScoggins it seems that it got detected by the bot in dotnet/perf-autofiling-issues#304, but it got reported a month after it had actually happened. Do we know why?
System.Text.Tests.Perf_StringBuilder.ctor_capacity(length: 100000)
|
|
would it be possible to share the traces? that would be very helpful |
sure! |
|
wow, both traces crashed perfview 😅 |
|
@adamsitnik Thanks for digging into this so deep!! |
jeffhandley
removed
the
needs-further-triage
|
I like that you recorded how you did the @adamsitnik so others can learn. |
|
Do we know how to reproduce this with a particular commit? I am wondering if I can bisect the change that caused the regression. |
|
@cshung the regression does not seem to have clear boundaries? If you go to the links above and zoom in, you can try to bisect but it seems it may be gradual. I made a quick attempt to bracket the time and got this range b1375a9...f59a132 that has a couple GC changes in, but they may well be irrelevant. |
|
Another option might be to attempt to add a perf test for "garbage collection of large arrays" and see whether it regressed. |
|
Might be worthwhile checking how things are with regions. |
|
The trends for and both seemed to have improved after the initial regression i.e., when regions in the GC space were re-introduced. I have taken a look at other trends related to StringBuilder from here and they either didn't regress or exhibited similar behavior where there was a regression followed by an improvement. Subsequent regressions seem unrelated to the GC. Should we close this issue? |
|
yeah should be ok to close if the current perf is comparable to .NET 6 numbers. |
|
Closing since we are back to (in some cases better than) the same numbers last year. Feel free to reopen if needed.
|
ghost
locked as resolved and limited conversation to collaborators
Repro:
git clone https://github.com/dotnet/performance.git py .\performance\scripts\benchmarks_ci.py -f net5.0 net6.0 --filter "System.Text.Tests.Perf_StringBuilder.ctor_capacity"It's not specific to any OS, it repros on both Windows and Unix:
https://pvscmdupload.blob.core.windows.net/reports/allTestHistory%2frefs%2fheads%2fmain_x64_Windows%2010.0.18362%2fSystem.Text.Tests.Perf_StringBuilder.ctor_capacity(length%3a%20100000).html
https://pvscmdupload.blob.core.windows.net/reports/allTestHistory%2frefs%2fheads%2fmain_x64_ubuntu%2018.04%2fSystem.Text.Tests.Perf_StringBuilder.ctor_capacity(length%3a%20100000).html
@DrewScoggins it seems that it got detected by the bot in dotnet/perf-autofiling-issues#304, but it got reported a month after it had actually happened. Do we know why?
System.Text.Tests.Perf_StringBuilder.ctor_capacity(length: 100000)
The text was updated successfully, but these errors were encountered: