[release/5.0] Fix reading performance counters on Windows ARM64 #46590
Conversation
* Remove dependency on PerfCounter.dll This dependency was unnecessary and only for calling through to pdh.dll. PerfCounter.dll is still needed for the consumption side of performance counters, but removing from the dependency from the .NET can help enable some scenarios like reading non-.NET counters. * Update conditions on performance counter tests. * Condition more perf counter tests
Update assembly version of PerformanceCounter package and build it in servicing. Also update the package baseline to ensure that Windows.Compatibility package gets the new version.
|
Tagging subscribers to this area: @tarekgh, @tommcdon, @pjanotti Issue DetailsBackport of #45142 to release/5.0 Fixes: #41144 Customer ImpactSystem performance counters cannot be read on Windows ARM64. Any access results Win32Exception due to failure to load perfcounter.dll module. Customers are blocked from using System.Diagnostics.PerformanceCounter on Windows ARM64. Regression?No, new platform TestingReenabled all failing unit tests, and better conditioned the tests to improve coverage. Outer-loop testing. RiskLow. The only risk here is that this may not be enough for some consumers of Performance Counters #46589. However the current fix should unblock most consumers.
|
| @@ -19,7 +19,7 @@ internal static partial class Libraries | |||
| internal const string Odbc32 = "odbc32.dll"; | |||
| internal const string Ole32 = "ole32.dll"; | |||
| internal const string OleAut32 = "oleaut32.dll"; | |||
| internal const string PerfCounter = "perfcounter.dll"; | |||
| internal const string Pdh = "pdh.dll"; | |||
There was a problem hiding this comment.
pdh [](start = 37, length = 3)
just wondering, does pdh.dll exist on all Windows skus we support? I guess so but wanted to ensure we looked at that.
There was a problem hiding this comment.
Yes. It was already being used by perfcounter.dll so it will exist everywhere this worked before. It's been present in Windows since XP/2k3. It's also part of nano and server core.
There was a problem hiding this comment.
perfcounter.dll (which it was using before) imports PdhFormatFromRaw value, which MSDN says back to Windows XP, eg
https://docs.microsoft.com/en-us/windows/win32/api/pdh/nf-pdh-pdhformatfromrawvalue
As for SKU's, the tests exclude only Nano server so it apparently is available on Server Core.
There was a problem hiding this comment.
Having said that, the old code presumably failed on Nano with Win32Exception here
and the new code presumably fails with EntrypointNotFoundException() ? Not sure that is important but just pointing it out.
There was a problem hiding this comment.
Oh, our comments crossed. Why do the tests exclude nano?
There was a problem hiding this comment.
I guess Nano not carrying the full framework which means the PerfCounter.dll is absent there. but maybe after this change we can try to enable the test there and look if it will succeed.
There was a problem hiding this comment.
Ah - of course. Well, that would be a happy result if it worked.
There was a problem hiding this comment.
My change did enable many test cases which read perf counters on nano. See many of the test cases which removed conditions completely. These tests are running and passing on nano.
I still conditioned a few tests which failed to write to perfcounters. I can't recall if I tried removing the write condition for nano or not. I can try this in master and see if we can enable more tests. It's somewhat irrelevant since folks won't be able to read the counters there due to lack of perfcounter.dll, but no harm in getting that coverage.
|
Package testing currently failing to restore Microsoft.NETCore.Platforms: It looks like we tie platforms package version to product version: Line 26 in 988fa3b We haven't made any changes to this package since GA: In 3.1 we maintained this independently: So I'm thinking we just dial back this property to |
|
I will cherry-pick ac5e2e3 to unblock CI on this. |
|
Mono test failure is #43981 |
leecow
added
Servicing-approved
|
networking failure #45204 |
ghost
locked as resolved and limited conversation to collaborators
Backport of #45142 to release/5.0
Fixes: #41144
Customer Impact
System performance counters cannot be read on Windows ARM64. Any access results Win32Exception due to failure to load perfcounter.dll module. Customers are blocked from using System.Diagnostics.PerformanceCounter on Windows ARM64.
Regression?
No, new platform
Testing
Reenabled all failing unit tests, and better conditioned the tests to improve coverage. Outer-loop testing.
Risk
Low. The only risk here is that this may not be enough for some consumers of Performance Counters #46589. However the current fix should unblock most consumers.