Runtime: [Perf] Widespread regressions after threadpool change

[DrewScoggins]

After https://github.com/dotnet/runtime/pull/43841, we are seeing widespread (about 40 tests) regress. Some of these are threading tests, but some seem to have little to do with threading, like https://github.com/DrewScoggins/performance-2/issues/3177. Below I have a link to a query that is all of the regressions and a few improvements that are linked to this change. We are seeing this across Windows x64, x86 and Ubuntu x64, and I will be updating this issue as we get the Windows x86, and Ubuntu x64 regressions categorized.

Regression Query - Windows x64

Improvement Query - Windows x64

[Dotnet-GitSync-Bot]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[kunalspathak]

cc: @kouvel , @mangod9

[mangod9]

Kount will take a look, this possibly might require to update the baselines since a startup hit might be causing some increase in per iteration times?

[kunalspathak]

Kount will take a look, this possibly might require to update the baselines since a startup hit might be causing some increase in per iteration times?

@adamsitnik

[AndyAyersMS]

BDN tests should generally be pretty well-insulated from startup effects. We also considered that the change might impact BDN itself, but that seems unlikely.

As Drew noted, a number of the regressions are in tests that don't seem to have anything to do with threading; this is quite puzzling. We also see instructions retired increases, so these regressions don't seem to be code alignment artifacts. Could be data alignment, but the regressions have been pretty stable. So it seems to suggest there must have been some path length changes in code that's not exclusively threading related.

[kouvel]

Part of the problem appears to be slower stack walking on the thread pool thread when performing a GC. Even though most of these tests don't use the thread pool, tiered compilation currently uses the thread pool to do its background work, and thread pool thread when waiting for work would have managed frames with GC refs. I could switch tiering to not use the thread pool, I was thinking about that anyway as it would give more control to choosing much CPU time to use in a stretch for rejits just using sleeps and without adding overhead from the thread pool queues and dispatching logic.

[mangod9]

would it be possible to validate that theory of "stackwalking on the threadpool thread" causing the regression, by disabling the portable threadpool via config?

[kouvel]

Yes it's been verified like that on a few cases, and it looks like that would be the cause for most if not all of the tests, I just haven't verified it on all of them yet, that's pending

[kouvel]

Although verifying like that would not tell whether it's the same issue for all of the tests. Would need profiles for each test.

[mangod9]

yeah validating against a few tests is fine, all are not required. We can create a tracking item to switch tiering to not use the TP. In the interim is there a way to rebase the tests to the new "baseline" -- @DrewScoggins ?

[DrewScoggins]

I am not sure exactly what you mean. We can take the PR and run it through the perf lab and generate a report to see if we are seeing improvements across the tests that we saw regressions in before. Or we can just test it locally on a few tests then check it in and then generate reports from the lab to make sure we got everything back.

[mangod9]

I meant the fix would probably take some time to implement, so wondering if we can rebase the perf baseline, so it catches any further regressions which might happen on top on this.

[DrewScoggins]

Oh, I understand now. The system already does this. So if we see any further regressions in this test we would flag them as a regression and report them.