[Perf] Regressions in System.Collections.TryGetValueFalse<String, String> #51258
Comments
DrewScoggins
added
os-linux
dotnet-issue-labeler
bot
added
area-System.Collections
untriaged
|
Tagging subscribers to this area: @eiriktsarpalis Issue DetailsRun Information
Regressions in System.Collections.TryGetValueFalse<String, String>
Reprogit clone https://github.com/dotnet/performance.git
python3 .\performance\scripts\benchmarks_ci.py -f netcoreapp5.0 --filter 'System.Collections.TryGetValueFalse<String, String>*'PayloadsHistogramSystem.Collections.TryGetValueFalse<String, String>.IDictionary(Size: 512)System.Collections.TryGetValueFalse<String, String>.Dictionary(Size: 512)DocsProfiling workflow for dotnet/runtime repository
|
_1.png)
_9.png)
Run Information
Improvemnts in System.Collections.ContainsKeyFalse<String, String>
Reprogit clone https://github.com/dotnet/performance.git
python3 .\performance\scripts\benchmarks_ci.py -f netcoreapp5.0 --filter 'System.Collections.ContainsKeyFalse<String, String>*'PayloadsHistogramSystem.Collections.ContainsKeyFalse<String, String>.IDictionary(Size: 512)System.Collections.ContainsKeyFalse<String, String>.Dictionary(Size: 512)DocsProfiling workflow for dotnet/runtime repository |
_1.png)
_9.png)
|
Looking at the full test trends for TryGetValue and ContainsKeyFalse, seems to show that when one gets faster the other get slower.
|
|
@DrewScoggins the zip archives linked above lose all file permissions (or else I'm doing it wrong). Makes it painful to download and then run the binaries. after an unzip: Can we instead get a zipped up tar archive? |
|
FWIW I can't repro the above regression with local builds on my unix box, which is why I wanted to grab the exact bits used by the runs. |
|
Those are the zips that we send to Helix, and we don't really have another place where we can easily get them. If I remember the only thing you had to |
|
If this is what you send, I wonder what helix does to work around this? Let me see if fixing corerun to be +x and all the others +r does the trick. |
|
It requires more futzing about than just that, not quite sure what BDN is doing... at any rate I have the same non-result with the downloaded builds as I did with the local builds. Could be my ancient HW I suppose. TryGetValueFalseBase, DownloadBenchmarkDotNet=v0.12.1.1521-nightly, OS=ubuntu 18.04
Intel Core i7-2720QM CPU 2.20GHz (Sandy Bridge), 1 CPU, 8 logical and 4 physical cores
.NET SDK=6.0.100-preview.3.21202.5
[Host] : .NET 6.0.0 (6.0.21.20104), X64 RyuJIT
Job-CXOBIO : .NET 6.0.0 (6.0.21.20502), X64 RyuJIT
PowerPlanMode=00000000-0000-0000-0000-000000000000 Arguments=/p:DebugType=portable Toolchain=CoreRun
IterationTime=250.0000 ms MaxIterationCount=20 MinIterationCount=15
WarmupCount=1
Base, LocalBenchmarkDotNet=v0.12.1.1521-nightly, OS=ubuntu 18.04
Intel Core i7-2720QM CPU 2.20GHz (Sandy Bridge), 1 CPU, 8 logical and 4 physical cores
.NET SDK=6.0.100-preview.3.21202.5
[Host] : .NET 6.0.0 (6.0.21.20104), X64 RyuJIT
Job-NDBUIM : .NET 6.0.0 (42.42.42.42424), X64 RyuJIT
PowerPlanMode=00000000-0000-0000-0000-000000000000 Arguments=/p:DebugType=portable Toolchain=CoreRun
IterationTime=250.0000 ms MaxIterationCount=20 MinIterationCount=15
WarmupCount=1
Diff, DownloadBenchmarkDotNet=v0.12.1.1521-nightly, OS=ubuntu 18.04
Intel Core i7-2720QM CPU 2.20GHz (Sandy Bridge), 1 CPU, 8 logical and 4 physical cores
.NET SDK=6.0.100-preview.3.21202.5
[Host] : .NET 6.0.0 (6.0.21.20104), X64 RyuJIT
Job-PLJKLO : .NET 6.0.0 (6.0.21.20602), X64 RyuJIT
PowerPlanMode=00000000-0000-0000-0000-000000000000 Arguments=/p:DebugType=portable Toolchain=CoreRun
IterationTime=250.0000 ms MaxIterationCount=20 MinIterationCount=15
WarmupCount=1
Diff, LocalBenchmarkDotNet=v0.12.1.1521-nightly, OS=ubuntu 18.04
Intel Core i7-2720QM CPU 2.20GHz (Sandy Bridge), 1 CPU, 8 logical and 4 physical cores
.NET SDK=6.0.100-preview.3.21202.5
[Host] : .NET 6.0.0 (6.0.21.20104), X64 RyuJIT
Job-KXUFTT : .NET 6.0.0 (42.42.42.42424), X64 RyuJIT
PowerPlanMode=00000000-0000-0000-0000-000000000000 Arguments=/p:DebugType=portable Toolchain=CoreRun
IterationTime=250.0000 ms MaxIterationCount=20 MinIterationCount=15
WarmupCount=1
|
|
Also note the same tests on windows x64 sped up at that same point: Windows
Ubuntu
(likewise for I don't suppose we have ETL/IR data for those windows runs...? |
|
Taking a fresh look, here's the recent history. We see 20% or so swings in perf. Also lately we seem to have reached a new low, but given history it's not clear how durable that is going to be. Working back in time, these jumps all seem to be correlated with PGO updates:
We should be able to use profiling to focus in on the key methods, and then check the optimization data we've gathered to see if it is fluctuating. Also worth noting, the dictionary case (which is testing the same exact code), shows the same pattern of fluctuation.
|
|
Profiling the test above (and filtering to just the "actual" timed runs done by BDN) Looking at codegen, the most likely method impacted by PGO is |
|
With current profile data, here are the PGO counts. the GDV sites in So we should look into the stability of these class profiles over time. |
|
Comparing the codegen for For the older data, we have and for the newer They largely agree, save for the BB16 -> BB13 edge, which has a much lower count in the second collection. After solving, this gives a higher weight for BB13, BB14, and BB16 (old profile below, new profile in the comment above) Impact of this on codegen is fairly minimal, we end up re-ordering some blocks at the end of the method. |
|
Similar diffs looking back to the May 21 codegen, edge weights differ leading to mainly different block layouts but otherwise "equivalent" code. Some of the layout diffs also come from shifting likelihoods in guarded devirtualization. |
|
On my box I'm able to repro about a 4% regression with the June 2 build, vs May 21 and June 7:
None of the other key methods have diffs, so evidently the block layout changes in Seems like if we fixed the class profile merge logic (see #48549 (comment)) that might lead to somewhat more stable profiles. Not sure what leads to the remaining count instability. Could perhaps be lost counter updates from concurrency but would not expect such large swings. |
jeffschwMSFT
removed
the
untriaged
danmoseley
added
area-CodeGen-coreclr
|
Updating area based on analysis above. |
|
These tests are quite sensitive to fine details of PGO and perf swings back and forth depending on exact block layout. Down the road we can likely improve block layout's algorithms to avoid being quite so sensitive. But I don't think we can address this for 6.0. So am going to move to future. |
|
Are there any characteristics of regressions caused by "fine details of PGO", changing PGO data etc that we can spot in the graphs for other regressions? It feels like when I suspect PGO data I'm just waving my hands. Eg., would it look like bimodal between builds, but stable within iterations on the same build? |
Yes, this (more or less): if you look up at #51258 (comment) you can see that the perf of the test switches between two levels over time, and the timing of the swings is correlated with PGO updates. There other other factors that can cause this sort of behavior (memory alignment of data, etc) so checking for the correlations with PGO updates is important. |
|
Is there a file in the repo that contains the PGO update, then I can correlate it's history (ie., how do I know when there was an update) |
Do we still see significant impact from this? I guess there are next steps left in dotnet/performance#1602 On that subject, I guess remaining work on code alignment in #43227 will have to be prioritized one way or another for next cycle too. |
|
If you look at the test history data from the lab you can select a particular perf jump and left-click on the before level and "set baseline" and do the same for the after level and set compare. Then you can look at the commit range between the two; you're looking at something like this and within there you'll see an "Update Dependencies" PR -- within that you'll see updates to eng/Version.Details.xml, and in particular this one line that updates the PGO version: - <Dependency Name="optimization.PGO.CoreCLR" Version="1.0.0-prerelease.21320.4">
+ <Dependency Name="optimization.PGO.CoreCLR" Version="1.0.0-prerelease.21329.4"> |
|
Ah - that's what I need. I'll check that next time. |
|
When looking at the last manual perf run for 6.0 I assumed that these benchmarks are just flaky: System.Collections.ContainsKeyFalse<Int32, Int32>.SortedList(Size: 512)
System.Collections.TryGetValueFalse<Int32, Int32>.SortedList(Size: 512)
But looking at the historical data it seems that we have slightly regressed
|
|
Stale issue. |
Run Information
Regressions in System.Collections.TryGetValueFalse<String, String>
![graph]
Historical Data in Reporting System
Repro
Payloads
Baseline
Compare
Histogram
System.Collections.TryGetValueFalse<String, String>.IDictionary(Size: 512)
System.Collections.TryGetValueFalse<String, String>.Dictionary(Size: 512)
Docs
Profiling workflow for dotnet/runtime repository
Benchmarking workflow for dotnet/runtime repository
category:performance
theme:benchmarks
The text was updated successfully, but these errors were encountered: