Repository navigation
Start time regressions on windows across many TE benchmarks #74315
Description
I noticed a lot of start-time and time-to-first-request regressions on Windows in various benchmarks, e.g.:
JsonMvc:
PlatformPlaintext:
PlaintextMvc:
What is even more interesting that DynamicPGO is now as fast as FullPGO:
it suggests that less methods are R2R'd (or R2R is broken)
Activity
EgorBo commentedon Aug 21, 2022
dotnet/aspnetcore commit range: dotnet/aspnetcore@14a3e98...87ef6f0
dotnet/runtime commit range: 08fdcb7...37193fd
EgorBo commentedon Aug 21, 2022
The only thing I can blame is #74279 - it bumps R2R format version from 7.1 to 8.0 (and MINIMUM_READYTORUN_MAJOR_VERSION from 6 to 8)
cc @davidwrighton
I also fetched output dirs for both data points and checked corelib via r2rdump - they have 7.1 and 8.0 version accordingly.
No idea how it could affect the benchmark on windows only - feels like aspnetcore binaries are prejitted with previous version of r2r format so runtime had to ignore r2r for them.
EgorBo commentedon Aug 21, 2022
Yes, e.g. Kestrel (
Microsoft.AspNetCore.Server.Kestrel.Core.dll) is R2R 7.1 while CoreLib is 8.0 for the "after" data point.From my understanding,
System.Private.CoreLib.dllis the only binary we prejit in dotnet/runtime using locally built crossgen, other bits are prejitted in dotnet/installer with whatever its global.json is - https://github.com/dotnet/installer/blob/main/global.json#L3 - so we need to update dotnet/installer to point to an SDK that produces 8.0 R2R schemeUPD: or, actually, crossgen version looks to be controlled by this version - https://github.com/dotnet/installer/blob/04eccb846748563880c407b91e009e13ed8ce495/eng/Versions.props#L80
EgorBo commentedon Aug 21, 2022
It actually feels a bit weird if my guesses are correct - it means when you download a daily SDK - all binaries except System.Private.CoreLib.dll are prejitted with some older crossgen, not the one you'd expect - it might explain some unexplainable regressions/improvements we observe time to time during dotnet/performance triage meetings - e.g. because newer R2R images bring updated static PGO data
mangod9 commentedon Aug 22, 2022
R2R version doesnt increment frequently. Agree that we are in a state where all previous crossgen2 built R2R binaries are going to be ignored. We need to ensure that asp.net builds with latest cg2 for RC1 to avoid these regressions.
mangod9 commentedon Aug 22, 2022
surprising this doesnt show up on Linux? Perhaps the runtime version is not updating yet for those runs?
sebastienros commentedon Aug 22, 2022
Also I changed the SDK feed and it's taking 8.0.* now, but the regression doesn't seem related. Might be better to check we get the same numbers with a 7.0 SDK (
--application.sdkVersion 7.0.100-rc.2.22419.9)mangod9 commentedon Aug 22, 2022
do we keep tracking 7 till GA? There are changes still being made there which would impact perf.
sebastienros commentedon Aug 22, 2022
We still take runtime and aspnet from 7 branches, just the SDK is updated so we are ready to run with net8.0 TFM when it's available (and runtime+aspnet).
ivdiazsa commentedon Aug 22, 2022
For the record, Linux runs have been also showing noticeable slowdowns.
mangod9 commentedon Aug 24, 2022
Windows startup seems to have normalized, but Linux hasn't yet. Is there a delay in when the sdk is updated for the two platforms on TE?
sebastienros commentedon Aug 24, 2022
The benchmarks take the same SDK version for both. Sometimes the charts might be delayed. But I checked the version numbers in the tooltips and they are up to date.
sebastienros commentedon Aug 24, 2022
Issue seems to be fixed. The discrepancy is due to some machines using the dotnet8 feeds for builds, which contain some rc1 builds that are older than the latest ones. If you check the runtime version on these benchmarks you'll see it's not going up because of that. The charts that are fixed have up-to-date runtime versions with the fix.
I am pushing a change to crank so that it will look on the other feeds for more recent versions.
mangod9 commentedon Aug 25, 2022
closing this since its an infra issue.