Revert "Improve cross-platform node discovery for reuse with NodeMode filtering"#13313
Revert "Improve cross-platform node discovery for reuse with NodeMode filtering"#13313JanProvaznik wants to merge 1 commit intomainfrom
Conversation
… filteri…" This reverts commit c48fb46.
|
confirmed also by RPS trace analyzer combase.dll!CoCreateInstanceEx |
There was a problem hiding this comment.
Pull request overview
This PR reverts PR #13256 ("Improve cross-platform node discovery for reuse with NodeMode filtering") because it caused a Visual Studio performance regression by loading WMI-related DLLs (wbemcomn.dll, wbemprox.dll, wbemsvc.dll, fastprox.dll, amsi.dll) on every MSBuild node provider initialization when node reuse was enabled.
Changes:
- Removes the WMI COM interop–based
TryGetCommandLine/Windows.GetCommandLineimplementation and related platform-specific (Linux/proc, macOS/BSD sysctl) command line retrieval code fromProcessExtensions.cs - Removes
NodeMode-based process filtering fromGetPossibleRunningNodesinNodeProviderOutOfProcBase.csand theExtractFromCommandLinehelper fromNodeModeHelper, replacing it with a simpler inline regex inDebugUtils.cs - Removes associated tests and cleans up
AllowUnsafeBlocks,partial classmodifiers, and[SupportedOSPlatformGuard]attributes that were no longer necessary
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/Shared/ProcessExtensions.cs |
Removes all WMI/Linux/macOS command-line retrieval logic; restores to KillTree-only class |
src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs |
Removes NodeMode-based process filtering from GetPossibleRunningNodes; reverts to plain process-name lookup |
src/Framework/NodeMode.cs |
Removes ExtractFromCommandLine, ReadOnlySpan<char> overload, and partial keyword from NodeModeHelper |
src/Shared/Debugging/DebugUtils.cs |
Inlines a new ScanNodeMode local function to replace the removed NodeModeHelper.ExtractFromCommandLine |
src/Framework/NativeMethods.cs |
Removes [SupportedOSPlatformGuard] attributes from IsBSD and IsOSX (no longer needed without platform-specific dispatch) |
src/Utilities.UnitTests/ProcessExtensions_Tests.cs |
Removes TryGetCommandLine-related tests; keeps KillTree test |
src/MSBuild.UnitTests/Microsoft.Build.CommandLine.UnitTests.csproj |
Removes AllowUnsafeBlocks (no longer needed) |
src/Tasks.UnitTests/Microsoft.Build.Tasks.UnitTests.csproj |
Adds ProcessExtensions.cs compile include (re-adding shared file reference) |
src/Framework.UnitTests/Microsoft.Build.Framework.UnitTests.csproj |
Adds ProcessExtensions.cs compile include |
src/Build.OM.UnitTests/Microsoft.Build.Engine.OM.UnitTests.csproj |
Adds ProcessExtensions.cs compile include |
|
@JanProvaznik per internal chat we'd like to instead of reverting simply ifdef the change so that it only appears in core msbuild for now. |
Reverts #13256
Causes VS perf regression in loaded DLL count scenarios.
Copilot analysis: