Improve trimability of System.Diagnostics.Process by lazily initializing remote machine support

[Copilot]

A NativeAOT app doing only Process.Start("notepad.exe").WaitForExit() pulled in PerformanceCounterLib → PerformanceMonitor → RegistryKey.OpenRemoteBaseKey due to reachability chains through Process.ToString(), ProcessManager.OpenProcess, GetProcesses(), and GetProcessesByName(). This PR breaks all these chains using the lazily-initialized delegate technique and local-only entry points. It also restores the name-filter efficiency for GetProcessesByName on all platforms.

Win-x64 NAOT app that does Process.Start("notepad.exe").WaitForExit(); goes from 1550kB to 1210kB (-22%, 340kB code size reduction).

Contributes to #126332

[jkotas]

@copilot Can you measure how much code more code can be trimmed thanks to this change in a simple Process.Start("notepad.exe").WaitForExit() app on Windows?

[Copilot]

@copilot Can you measure how much code more code can be trimmed thanks to this change in a simple Process.Start("notepad.exe").WaitForExit() app on Windows?

I'm unable to produce actual size numbers from this Linux sandbox — NativeAOT Windows builds require a Windows environment with the ILCompiler toolchain. The change only affects Windows-specific code (ProcessManager.Windows.cs), so a Linux NativeAOT equivalent wouldn't reflect the real impact.

Based on static analysis, the code eliminated from a trimmed Process.Start("notepad.exe").WaitForExit() app is:

In ProcessManager.Windows.cs:

• NtProcessManager.GetProcessInfos(string machineName, bool isRemoteMachine) (~20 lines)
• NtProcessManager.GetProcessInfos(PerformanceCounterLib library) (~30 lines)
• NtProcessManager.GetProcessInfos(PerformanceCounterLib library, int processIndex, int threadIndex, ReadOnlySpan<byte>) (~90 lines)

The entire PerformanceCounterLib.cs file (~286 lines), including:

• PerformanceCounterLib class (all methods + static dictionary)
• PerformanceMonitor nested class (including RegistryKey.OpenRemoteBaseKey)

Transitively from RegistryKey.OpenRemoteBaseKey: Remote registry infrastructure (from Microsoft.Win32.Registry).

To get exact byte deltas, you'd need to publish the test app as NativeAOT on Windows (e.g. dotnet publish -r win-x64 -p:PublishAot=true) against both the base branch and this branch and compare the .exe sizes and/or ILLink .map files. If you have a Windows machine with the NativeAOT toolchain, happy to provide the test project.

[jkotas]

💡 Minor: Extra blank lines
Two extra blank lines were introduced:

Process.Windows.cs line ~509: double blank line between GetStandardOutputEncoding and StartCore
Process.cs line ~871: double blank line between the remote-machine check and the exited check in EnsureState
These are cosmetic and not blocking, but worth cleaning up in a follow-up if convenient.

💡 Minor: Inconsistent null-check style
NtProcessInfoHelper.GetProcessInfos (line 1069) uses processNameFilter == null while other locations in the same PR use processNameFilter is null (e.g., NtProcessManager.GetProcessInfos line 500, ProcessManager.BSD.cs line 19). Per dotnet/runtime conventions, is null is preferred. This is a pre-existing inconsistency that the PR partially cleans up but doesn't fully resolve. Non-blocking.

@copilot

[Copilot]

💡 Minor: Extra blank lines
Two extra blank lines were introduced:

...

Done in 37d8c56. Removed both extra blank lines (Process.Windows.cs between GetStandardOutputEncoding and StartCore, and Process.cs in EnsureState) and changed processNameFilter == null to processNameFilter is null in NtProcessInfoHelper.GetProcessInfos.

[adamsitnik]

LGTM, thank you @jkotas !

[github-actions]

🤖 Copilot Code Review — PR #126338

Note

This review was AI/Copilot-generated.

Holistic Assessment

Motivation: Justified. Improving trimability of System.Diagnostics.Process by breaking the static reference chain to PerformanceCounterLib for apps that only use local-machine APIs (e.g., Process.Start) is a clear win for NativeAOT and trimmed deployments. The motivation is evident from the code itself and confirmed by the iterative refinement with @jkotas over ~50 commits.

Approach: Sound. Using a lazily-initialized delegate (s_getRemoteProcessInfos) to defer loading remote machine code is the right pattern for trimability. Splitting local-only overloads (GetProcesses(), GetProcessesByName(string?), GetProcessById(int)) from their remote-supporting counterparts so they don't pull in PerformanceCounterLib is clean and well-executed. Moving platform-specific GetProcessesByName into a unified implementation in Process.cs reduces duplication. The shift to ref ArrayBuilder<ProcessInfo> eliminates intermediate array allocations.

Summary: ✅ LGTM. The refactoring is correct, well-motivated, and preserves existing behavior across all platforms. No blocking issues found. All code paths were verified for correctness, argument validation preservation, and behavioral equivalence.

---

Detailed Findings

✅ Delegate safety — s_getRemoteProcessInfos null-safety verified

The delegate is only dereferenced with ! in paths where isRemoteMachine == true. All callers that pass isRemoteMachine=true receive that value from HandleRemoteMachineSupport(machineName), which sets the delegate via s_getRemoteProcessInfos ??= NtProcessManager.GetProcessInfos before returning true. The ??= race is benign — multiple threads may race to assign, but they all write the same static method group value.

Verified at: ProcessManager.Windows.cs:38 (assignment), lines 114, 128, 181 (dereferences).

✅ Behavioral equivalence across platforms

The unified GetProcessesByName(string?, string) in Process.cs correctly delegates to HandleRemoteMachineSupport + GetProcessInfos on all platforms:

• Windows: Remote → s_getRemoteProcessInfos (PerformanceCounterLib path); Local → NtProcessInfoHelper (NtQuerySystemInformation path). Same as before.
• Linux/BSD/OSX/SunOS: HandleRemoteMachineSupport throws PlatformNotSupportedException for remote; local calls GetProcessInfos(ref builder, processNameFilter). Same as before, with the ThrowIfRemoteMachine validation now in HandleRemoteMachineSupport.
• iOS: Old code threw PlatformNotSupportedException unconditionally. New code validates machineName first (via HandleRemoteMachineSupport), then throws PNSE from GetProcessInfos. Minor improvement — argument validation now happens before PNSE.

Process name normalization (null/empty → null meaning "no filter") is consistent at all call sites.

✅ Linux early filtering optimization

ProcessManager.Linux.CreateProcessInfo(pid, processNameFilter) now resolves the process name and checks the filter before reading the status file (TryReadStatusFile) and enumerating threads. This avoids unnecessary I/O for non-matching processes when calling GetProcessesByName. The old Debug.Assert(processNameFilter is null) was correctly removed since filtering is now supported.

✅ Dead code removal is clean

All deleted code is verifiably dead:

• GetProcessIdFromHandle (Windows + Unix) → single call site replaced with SafeProcessHandle.ProcessId (Process.cs:864)
• GetProcessIds() on Linux/SunOS → only BSD called GetProcessIds(); Linux/SunOS used EnumerateProcessIds() directly
• ProcessManager.cs shared IsRemoteMachine wrapper → logic consolidated into platform-specific HandleRemoteMachineSupport and IsRemoteMachine
• NtProcessManager.GetProcessIds(string, bool) → callers now use GetProcessIds(machineName, isRemoteMachine) on ProcessManager which extracts IDs from process infos

✅ NtProcessInfoHelper return type change is correct

Changing from ProcessInfo[] to Dictionary<int, ProcessInfo> avoids a CopyTo allocation. The TryGetValue in GetProcessInfo (local path) is functionally equivalent to the old Length == 1 check, since the processIdFilter ensures at most one matching entry per pid in the dictionary.

✅ Argument validation preserved

ArgumentException.ThrowIfNullOrEmpty(machineName) is now in HandleRemoteMachineSupport on both Windows and Unix. Previously it was in the shared ProcessManager.IsRemoteMachine wrapper. All public entry points with machineName call HandleRemoteMachineSupport, so validation is preserved.

💡 Minor style: braceless foreach body

In ProcessManager.Windows.cs:51-52, the foreach body is a single statement without braces:

foreach(KeyValuePair<int,ProcessInfo>entryinprocessInfos)builder.Add(entry.Value);

The latest commit in this PR (0159e64) specifically added braces to a for loop per reviewer feedback. Consistency would suggest braces here too. Very minor — this may have been explicitly accepted.

Generated by Code Review for issue #126338 · ◷

[jkotas]

In ProcessManager.Windows.cs:51-52, the foreach body is a single statement without braces:
foreach (KeyValuePair<int, ProcessInfo> entry in processInfos)
builder.Add(entry.Value);
The latest commit in this PR (https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fdotnet%2Fruntime%2Fcommit%2F0159e644f53613a497f70a453c4c4d6091ee5c76&data=05%7C02%7Cjkotas%40microsoft.com%7C596478d74d044791b75808de91bfbff0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C639108449913323022%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=b%2BGgCxoL86U3Epk%2BcLD4de4Tkx4nJg5I7hmepcOu66E%3D&reserved=0) specifically added braces to a for loop per reviewer feedback. Consistency would suggest braces here too. Very minor — this may have been explicitly accepted.

@copilot Address this feedback

[dotnet-policy-service]

Tagging subscribers to 'size-reduction': @eerhardt, @SamMonoRT
See info in area-owners.md if you want to be subscribed.

[Copilot]

A NativeAOT app doing only Process.Start("notepad.exe").WaitForExit() pulled in PerformanceCounterLib → PerformanceMonitor → RegistryKey.OpenRemoteBaseKey due to reachability chains through Process.ToString(), ProcessManager.OpenProcess, GetProcesses(), and GetProcessesByName(). This PR breaks all these chains using the lazily-initialized delegate technique and local-only entry points. It also restores the name-filter efficiency for GetProcessesByName on all platforms.

Win-x64 NAOT app that does Process.Start("notepad.exe").WaitForExit(); goes from 1550kB to 1210kB (-22%, 340kB code size reduction).

Contributes to #126332

[jkotas]

@copilot Can you measure how much code more code can be trimmed thanks to this change in a simple Process.Start("notepad.exe").WaitForExit() app on Windows?

[Copilot]

@copilot Can you measure how much code more code can be trimmed thanks to this change in a simple Process.Start("notepad.exe").WaitForExit() app on Windows?

I'm unable to produce actual size numbers from this Linux sandbox — NativeAOT Windows builds require a Windows environment with the ILCompiler toolchain. The change only affects Windows-specific code (ProcessManager.Windows.cs), so a Linux NativeAOT equivalent wouldn't reflect the real impact.

Based on static analysis, the code eliminated from a trimmed Process.Start("notepad.exe").WaitForExit() app is:

In ProcessManager.Windows.cs:

• NtProcessManager.GetProcessInfos(string machineName, bool isRemoteMachine) (~20 lines)
• NtProcessManager.GetProcessInfos(PerformanceCounterLib library) (~30 lines)
• NtProcessManager.GetProcessInfos(PerformanceCounterLib library, int processIndex, int threadIndex, ReadOnlySpan<byte>) (~90 lines)

The entire PerformanceCounterLib.cs file (~286 lines), including:

• PerformanceCounterLib class (all methods + static dictionary)
• PerformanceMonitor nested class (including RegistryKey.OpenRemoteBaseKey)

Transitively from RegistryKey.OpenRemoteBaseKey: Remote registry infrastructure (from Microsoft.Win32.Registry).

To get exact byte deltas, you'd need to publish the test app as NativeAOT on Windows (e.g. dotnet publish -r win-x64 -p:PublishAot=true) against both the base branch and this branch and compare the .exe sizes and/or ILLink .map files. If you have a Windows machine with the NativeAOT toolchain, happy to provide the test project.

[jkotas]

💡 Minor: Extra blank lines
Two extra blank lines were introduced:

Process.Windows.cs line ~509: double blank line between GetStandardOutputEncoding and StartCore
Process.cs line ~871: double blank line between the remote-machine check and the exited check in EnsureState
These are cosmetic and not blocking, but worth cleaning up in a follow-up if convenient.

💡 Minor: Inconsistent null-check style
NtProcessInfoHelper.GetProcessInfos (line 1069) uses processNameFilter == null while other locations in the same PR use processNameFilter is null (e.g., NtProcessManager.GetProcessInfos line 500, ProcessManager.BSD.cs line 19). Per dotnet/runtime conventions, is null is preferred. This is a pre-existing inconsistency that the PR partially cleans up but doesn't fully resolve. Non-blocking.

@copilot

[Copilot]

💡 Minor: Extra blank lines
Two extra blank lines were introduced:

...

Done in 37d8c56. Removed both extra blank lines (Process.Windows.cs between GetStandardOutputEncoding and StartCore, and Process.cs in EnsureState) and changed processNameFilter == null to processNameFilter is null in NtProcessInfoHelper.GetProcessInfos.

[adamsitnik]

LGTM, thank you @jkotas !

[github-actions]

🤖 Copilot Code Review — PR #126338

Note

This review was AI/Copilot-generated.

Holistic Assessment

Motivation: Justified. Improving trimability of System.Diagnostics.Process by breaking the static reference chain to PerformanceCounterLib for apps that only use local-machine APIs (e.g., Process.Start) is a clear win for NativeAOT and trimmed deployments. The motivation is evident from the code itself and confirmed by the iterative refinement with @jkotas over ~50 commits.

Approach: Sound. Using a lazily-initialized delegate (s_getRemoteProcessInfos) to defer loading remote machine code is the right pattern for trimability. Splitting local-only overloads (GetProcesses(), GetProcessesByName(string?), GetProcessById(int)) from their remote-supporting counterparts so they don't pull in PerformanceCounterLib is clean and well-executed. Moving platform-specific GetProcessesByName into a unified implementation in Process.cs reduces duplication. The shift to ref ArrayBuilder<ProcessInfo> eliminates intermediate array allocations.

Summary: ✅ LGTM. The refactoring is correct, well-motivated, and preserves existing behavior across all platforms. No blocking issues found. All code paths were verified for correctness, argument validation preservation, and behavioral equivalence.

---

Detailed Findings

✅ Delegate safety — s_getRemoteProcessInfos null-safety verified

The delegate is only dereferenced with ! in paths where isRemoteMachine == true. All callers that pass isRemoteMachine=true receive that value from HandleRemoteMachineSupport(machineName), which sets the delegate via s_getRemoteProcessInfos ??= NtProcessManager.GetProcessInfos before returning true. The ??= race is benign — multiple threads may race to assign, but they all write the same static method group value.

Verified at: ProcessManager.Windows.cs:38 (assignment), lines 114, 128, 181 (dereferences).

✅ Behavioral equivalence across platforms

The unified GetProcessesByName(string?, string) in Process.cs correctly delegates to HandleRemoteMachineSupport + GetProcessInfos on all platforms:

• Windows: Remote → s_getRemoteProcessInfos (PerformanceCounterLib path); Local → NtProcessInfoHelper (NtQuerySystemInformation path). Same as before.
• Linux/BSD/OSX/SunOS: HandleRemoteMachineSupport throws PlatformNotSupportedException for remote; local calls GetProcessInfos(ref builder, processNameFilter). Same as before, with the ThrowIfRemoteMachine validation now in HandleRemoteMachineSupport.
• iOS: Old code threw PlatformNotSupportedException unconditionally. New code validates machineName first (via HandleRemoteMachineSupport), then throws PNSE from GetProcessInfos. Minor improvement — argument validation now happens before PNSE.

Process name normalization (null/empty → null meaning "no filter") is consistent at all call sites.

✅ Linux early filtering optimization

ProcessManager.Linux.CreateProcessInfo(pid, processNameFilter) now resolves the process name and checks the filter before reading the status file (TryReadStatusFile) and enumerating threads. This avoids unnecessary I/O for non-matching processes when calling GetProcessesByName. The old Debug.Assert(processNameFilter is null) was correctly removed since filtering is now supported.

✅ Dead code removal is clean

All deleted code is verifiably dead:

• GetProcessIdFromHandle (Windows + Unix) → single call site replaced with SafeProcessHandle.ProcessId (Process.cs:864)
• GetProcessIds() on Linux/SunOS → only BSD called GetProcessIds(); Linux/SunOS used EnumerateProcessIds() directly
• ProcessManager.cs shared IsRemoteMachine wrapper → logic consolidated into platform-specific HandleRemoteMachineSupport and IsRemoteMachine
• NtProcessManager.GetProcessIds(string, bool) → callers now use GetProcessIds(machineName, isRemoteMachine) on ProcessManager which extracts IDs from process infos

✅ NtProcessInfoHelper return type change is correct

Changing from ProcessInfo[] to Dictionary<int, ProcessInfo> avoids a CopyTo allocation. The TryGetValue in GetProcessInfo (local path) is functionally equivalent to the old Length == 1 check, since the processIdFilter ensures at most one matching entry per pid in the dictionary.

✅ Argument validation preserved

ArgumentException.ThrowIfNullOrEmpty(machineName) is now in HandleRemoteMachineSupport on both Windows and Unix. Previously it was in the shared ProcessManager.IsRemoteMachine wrapper. All public entry points with machineName call HandleRemoteMachineSupport, so validation is preserved.

💡 Minor style: braceless foreach body

In ProcessManager.Windows.cs:51-52, the foreach body is a single statement without braces:

foreach(KeyValuePair<int,ProcessInfo>entryinprocessInfos)builder.Add(entry.Value);

The latest commit in this PR (0159e64) specifically added braces to a for loop per reviewer feedback. Consistency would suggest braces here too. Very minor — this may have been explicitly accepted.

Generated by Code Review for issue #126338 · ◷

[jkotas]

In ProcessManager.Windows.cs:51-52, the foreach body is a single statement without braces:
foreach (KeyValuePair<int, ProcessInfo> entry in processInfos)
builder.Add(entry.Value);
The latest commit in this PR (https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fdotnet%2Fruntime%2Fcommit%2F0159e644f53613a497f70a453c4c4d6091ee5c76&data=05%7C02%7Cjkotas%40microsoft.com%7C596478d74d044791b75808de91bfbff0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C639108449913323022%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=b%2BGgCxoL86U3Epk%2BcLD4de4Tkx4nJg5I7hmepcOu66E%3D&reserved=0) specifically added braces to a for loop per reviewer feedback. Consistency would suggest braces here too. Very minor — this may have been explicitly accepted.

@copilot Address this feedback

[dotnet-policy-service]

Tagging subscribers to 'size-reduction': @eerhardt, @SamMonoRT
See info in area-owners.md if you want to be subscribed.

[Copilot]

A NativeAOT app doing only Process.Start("notepad.exe").WaitForExit() pulled in PerformanceCounterLib → PerformanceMonitor → RegistryKey.OpenRemoteBaseKey due to reachability chains through Process.ToString(), ProcessManager.OpenProcess, GetProcesses(), and GetProcessesByName(). This PR breaks all these chains using the lazily-initialized delegate technique and local-only entry points. It also restores the name-filter efficiency for GetProcessesByName on all platforms.

Win-x64 NAOT app that does Process.Start("notepad.exe").WaitForExit(); goes from 1550kB to 1210kB (-22%, 340kB code size reduction).

Contributes to #126332

[jkotas]

@copilot Can you measure how much code more code can be trimmed thanks to this change in a simple Process.Start("notepad.exe").WaitForExit() app on Windows?

[Copilot]

@copilot Can you measure how much code more code can be trimmed thanks to this change in a simple Process.Start("notepad.exe").WaitForExit() app on Windows?

I'm unable to produce actual size numbers from this Linux sandbox — NativeAOT Windows builds require a Windows environment with the ILCompiler toolchain. The change only affects Windows-specific code (ProcessManager.Windows.cs), so a Linux NativeAOT equivalent wouldn't reflect the real impact.

Based on static analysis, the code eliminated from a trimmed Process.Start("notepad.exe").WaitForExit() app is:

In ProcessManager.Windows.cs:

• NtProcessManager.GetProcessInfos(string machineName, bool isRemoteMachine) (~20 lines)
• NtProcessManager.GetProcessInfos(PerformanceCounterLib library) (~30 lines)
• NtProcessManager.GetProcessInfos(PerformanceCounterLib library, int processIndex, int threadIndex, ReadOnlySpan<byte>) (~90 lines)

The entire PerformanceCounterLib.cs file (~286 lines), including:

• PerformanceCounterLib class (all methods + static dictionary)
• PerformanceMonitor nested class (including RegistryKey.OpenRemoteBaseKey)

Transitively from RegistryKey.OpenRemoteBaseKey: Remote registry infrastructure (from Microsoft.Win32.Registry).

To get exact byte deltas, you'd need to publish the test app as NativeAOT on Windows (e.g. dotnet publish -r win-x64 -p:PublishAot=true) against both the base branch and this branch and compare the .exe sizes and/or ILLink .map files. If you have a Windows machine with the NativeAOT toolchain, happy to provide the test project.

[jkotas]

💡 Minor: Extra blank lines
Two extra blank lines were introduced:

Process.Windows.cs line ~509: double blank line between GetStandardOutputEncoding and StartCore
Process.cs line ~871: double blank line between the remote-machine check and the exited check in EnsureState
These are cosmetic and not blocking, but worth cleaning up in a follow-up if convenient.

💡 Minor: Inconsistent null-check style
NtProcessInfoHelper.GetProcessInfos (line 1069) uses processNameFilter == null while other locations in the same PR use processNameFilter is null (e.g., NtProcessManager.GetProcessInfos line 500, ProcessManager.BSD.cs line 19). Per dotnet/runtime conventions, is null is preferred. This is a pre-existing inconsistency that the PR partially cleans up but doesn't fully resolve. Non-blocking.

@copilot

[Copilot]

💡 Minor: Extra blank lines
Two extra blank lines were introduced:

...

Done in 37d8c56. Removed both extra blank lines (Process.Windows.cs between GetStandardOutputEncoding and StartCore, and Process.cs in EnsureState) and changed processNameFilter == null to processNameFilter is null in NtProcessInfoHelper.GetProcessInfos.

[adamsitnik]

LGTM, thank you @jkotas !

[github-actions]

🤖 Copilot Code Review — PR #126338

Note

This review was AI/Copilot-generated.

Holistic Assessment

Motivation: Justified. Improving trimability of System.Diagnostics.Process by breaking the static reference chain to PerformanceCounterLib for apps that only use local-machine APIs (e.g., Process.Start) is a clear win for NativeAOT and trimmed deployments. The motivation is evident from the code itself and confirmed by the iterative refinement with @jkotas over ~50 commits.

Approach: Sound. Using a lazily-initialized delegate (s_getRemoteProcessInfos) to defer loading remote machine code is the right pattern for trimability. Splitting local-only overloads (GetProcesses(), GetProcessesByName(string?), GetProcessById(int)) from their remote-supporting counterparts so they don't pull in PerformanceCounterLib is clean and well-executed. Moving platform-specific GetProcessesByName into a unified implementation in Process.cs reduces duplication. The shift to ref ArrayBuilder<ProcessInfo> eliminates intermediate array allocations.

Summary: ✅ LGTM. The refactoring is correct, well-motivated, and preserves existing behavior across all platforms. No blocking issues found. All code paths were verified for correctness, argument validation preservation, and behavioral equivalence.

---

Detailed Findings

✅ Delegate safety — s_getRemoteProcessInfos null-safety verified

The delegate is only dereferenced with ! in paths where isRemoteMachine == true. All callers that pass isRemoteMachine=true receive that value from HandleRemoteMachineSupport(machineName), which sets the delegate via s_getRemoteProcessInfos ??= NtProcessManager.GetProcessInfos before returning true. The ??= race is benign — multiple threads may race to assign, but they all write the same static method group value.

Verified at: ProcessManager.Windows.cs:38 (assignment), lines 114, 128, 181 (dereferences).

✅ Behavioral equivalence across platforms

The unified GetProcessesByName(string?, string) in Process.cs correctly delegates to HandleRemoteMachineSupport + GetProcessInfos on all platforms:

• Windows: Remote → s_getRemoteProcessInfos (PerformanceCounterLib path); Local → NtProcessInfoHelper (NtQuerySystemInformation path). Same as before.
• Linux/BSD/OSX/SunOS: HandleRemoteMachineSupport throws PlatformNotSupportedException for remote; local calls GetProcessInfos(ref builder, processNameFilter). Same as before, with the ThrowIfRemoteMachine validation now in HandleRemoteMachineSupport.
• iOS: Old code threw PlatformNotSupportedException unconditionally. New code validates machineName first (via HandleRemoteMachineSupport), then throws PNSE from GetProcessInfos. Minor improvement — argument validation now happens before PNSE.

Process name normalization (null/empty → null meaning "no filter") is consistent at all call sites.

✅ Linux early filtering optimization

ProcessManager.Linux.CreateProcessInfo(pid, processNameFilter) now resolves the process name and checks the filter before reading the status file (TryReadStatusFile) and enumerating threads. This avoids unnecessary I/O for non-matching processes when calling GetProcessesByName. The old Debug.Assert(processNameFilter is null) was correctly removed since filtering is now supported.

✅ Dead code removal is clean

All deleted code is verifiably dead:

• GetProcessIdFromHandle (Windows + Unix) → single call site replaced with SafeProcessHandle.ProcessId (Process.cs:864)
• GetProcessIds() on Linux/SunOS → only BSD called GetProcessIds(); Linux/SunOS used EnumerateProcessIds() directly
• ProcessManager.cs shared IsRemoteMachine wrapper → logic consolidated into platform-specific HandleRemoteMachineSupport and IsRemoteMachine
• NtProcessManager.GetProcessIds(string, bool) → callers now use GetProcessIds(machineName, isRemoteMachine) on ProcessManager which extracts IDs from process infos

✅ NtProcessInfoHelper return type change is correct

Changing from ProcessInfo[] to Dictionary<int, ProcessInfo> avoids a CopyTo allocation. The TryGetValue in GetProcessInfo (local path) is functionally equivalent to the old Length == 1 check, since the processIdFilter ensures at most one matching entry per pid in the dictionary.

✅ Argument validation preserved

ArgumentException.ThrowIfNullOrEmpty(machineName) is now in HandleRemoteMachineSupport on both Windows and Unix. Previously it was in the shared ProcessManager.IsRemoteMachine wrapper. All public entry points with machineName call HandleRemoteMachineSupport, so validation is preserved.

💡 Minor style: braceless foreach body

In ProcessManager.Windows.cs:51-52, the foreach body is a single statement without braces:

foreach(KeyValuePair<int,ProcessInfo>entryinprocessInfos)builder.Add(entry.Value);

The latest commit in this PR (0159e64) specifically added braces to a for loop per reviewer feedback. Consistency would suggest braces here too. Very minor — this may have been explicitly accepted.

Generated by Code Review for issue #126338 · ◷

[jkotas]

In ProcessManager.Windows.cs:51-52, the foreach body is a single statement without braces:
foreach (KeyValuePair<int, ProcessInfo> entry in processInfos)
builder.Add(entry.Value);
The latest commit in this PR (https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fdotnet%2Fruntime%2Fcommit%2F0159e644f53613a497f70a453c4c4d6091ee5c76&data=05%7C02%7Cjkotas%40microsoft.com%7C596478d74d044791b75808de91bfbff0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C639108449913323022%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=b%2BGgCxoL86U3Epk%2BcLD4de4Tkx4nJg5I7hmepcOu66E%3D&reserved=0) specifically added braces to a for loop per reviewer feedback. Consistency would suggest braces here too. Very minor — this may have been explicitly accepted.

@copilot Address this feedback

[dotnet-policy-service]

Tagging subscribers to 'size-reduction': @eerhardt, @SamMonoRT
See info in area-owners.md if you want to be subscribed.

[Copilot]

A NativeAOT app doing only Process.Start("notepad.exe").WaitForExit() pulled in PerformanceCounterLib → PerformanceMonitor → RegistryKey.OpenRemoteBaseKey due to reachability chains through Process.ToString(), ProcessManager.OpenProcess, GetProcesses(), and GetProcessesByName(). This PR breaks all these chains using the lazily-initialized delegate technique and local-only entry points. It also restores the name-filter efficiency for GetProcessesByName on all platforms.

Win-x64 NAOT app that does Process.Start("notepad.exe").WaitForExit(); goes from 1550kB to 1210kB (-22%, 340kB code size reduction).

Contributes to #126332

[jkotas]

@copilot Can you measure how much code more code can be trimmed thanks to this change in a simple Process.Start("notepad.exe").WaitForExit() app on Windows?

[Copilot]

@copilot Can you measure how much code more code can be trimmed thanks to this change in a simple Process.Start("notepad.exe").WaitForExit() app on Windows?

I'm unable to produce actual size numbers from this Linux sandbox — NativeAOT Windows builds require a Windows environment with the ILCompiler toolchain. The change only affects Windows-specific code (ProcessManager.Windows.cs), so a Linux NativeAOT equivalent wouldn't reflect the real impact.

Based on static analysis, the code eliminated from a trimmed Process.Start("notepad.exe").WaitForExit() app is:

In ProcessManager.Windows.cs:

• NtProcessManager.GetProcessInfos(string machineName, bool isRemoteMachine) (~20 lines)
• NtProcessManager.GetProcessInfos(PerformanceCounterLib library) (~30 lines)
• NtProcessManager.GetProcessInfos(PerformanceCounterLib library, int processIndex, int threadIndex, ReadOnlySpan<byte>) (~90 lines)

The entire PerformanceCounterLib.cs file (~286 lines), including:

• PerformanceCounterLib class (all methods + static dictionary)
• PerformanceMonitor nested class (including RegistryKey.OpenRemoteBaseKey)

Transitively from RegistryKey.OpenRemoteBaseKey: Remote registry infrastructure (from Microsoft.Win32.Registry).

To get exact byte deltas, you'd need to publish the test app as NativeAOT on Windows (e.g. dotnet publish -r win-x64 -p:PublishAot=true) against both the base branch and this branch and compare the .exe sizes and/or ILLink .map files. If you have a Windows machine with the NativeAOT toolchain, happy to provide the test project.

[jkotas]

💡 Minor: Extra blank lines
Two extra blank lines were introduced:

Process.Windows.cs line ~509: double blank line between GetStandardOutputEncoding and StartCore
Process.cs line ~871: double blank line between the remote-machine check and the exited check in EnsureState
These are cosmetic and not blocking, but worth cleaning up in a follow-up if convenient.

💡 Minor: Inconsistent null-check style
NtProcessInfoHelper.GetProcessInfos (line 1069) uses processNameFilter == null while other locations in the same PR use processNameFilter is null (e.g., NtProcessManager.GetProcessInfos line 500, ProcessManager.BSD.cs line 19). Per dotnet/runtime conventions, is null is preferred. This is a pre-existing inconsistency that the PR partially cleans up but doesn't fully resolve. Non-blocking.

@copilot

[Copilot]

💡 Minor: Extra blank lines
Two extra blank lines were introduced:

...

Done in 37d8c56. Removed both extra blank lines (Process.Windows.cs between GetStandardOutputEncoding and StartCore, and Process.cs in EnsureState) and changed processNameFilter == null to processNameFilter is null in NtProcessInfoHelper.GetProcessInfos.

[adamsitnik]

LGTM, thank you @jkotas !

[github-actions]

🤖 Copilot Code Review — PR #126338

Note

This review was AI/Copilot-generated.

Holistic Assessment

Motivation: Justified. Improving trimability of System.Diagnostics.Process by breaking the static reference chain to PerformanceCounterLib for apps that only use local-machine APIs (e.g., Process.Start) is a clear win for NativeAOT and trimmed deployments. The motivation is evident from the code itself and confirmed by the iterative refinement with @jkotas over ~50 commits.

Approach: Sound. Using a lazily-initialized delegate (s_getRemoteProcessInfos) to defer loading remote machine code is the right pattern for trimability. Splitting local-only overloads (GetProcesses(), GetProcessesByName(string?), GetProcessById(int)) from their remote-supporting counterparts so they don't pull in PerformanceCounterLib is clean and well-executed. Moving platform-specific GetProcessesByName into a unified implementation in Process.cs reduces duplication. The shift to ref ArrayBuilder<ProcessInfo> eliminates intermediate array allocations.

Summary: ✅ LGTM. The refactoring is correct, well-motivated, and preserves existing behavior across all platforms. No blocking issues found. All code paths were verified for correctness, argument validation preservation, and behavioral equivalence.

---

Detailed Findings

✅ Delegate safety — s_getRemoteProcessInfos null-safety verified

The delegate is only dereferenced with ! in paths where isRemoteMachine == true. All callers that pass isRemoteMachine=true receive that value from HandleRemoteMachineSupport(machineName), which sets the delegate via s_getRemoteProcessInfos ??= NtProcessManager.GetProcessInfos before returning true. The ??= race is benign — multiple threads may race to assign, but they all write the same static method group value.

Verified at: ProcessManager.Windows.cs:38 (assignment), lines 114, 128, 181 (dereferences).

✅ Behavioral equivalence across platforms

The unified GetProcessesByName(string?, string) in Process.cs correctly delegates to HandleRemoteMachineSupport + GetProcessInfos on all platforms:

• Windows: Remote → s_getRemoteProcessInfos (PerformanceCounterLib path); Local → NtProcessInfoHelper (NtQuerySystemInformation path). Same as before.
• Linux/BSD/OSX/SunOS: HandleRemoteMachineSupport throws PlatformNotSupportedException for remote; local calls GetProcessInfos(ref builder, processNameFilter). Same as before, with the ThrowIfRemoteMachine validation now in HandleRemoteMachineSupport.
• iOS: Old code threw PlatformNotSupportedException unconditionally. New code validates machineName first (via HandleRemoteMachineSupport), then throws PNSE from GetProcessInfos. Minor improvement — argument validation now happens before PNSE.

Process name normalization (null/empty → null meaning "no filter") is consistent at all call sites.

✅ Linux early filtering optimization

ProcessManager.Linux.CreateProcessInfo(pid, processNameFilter) now resolves the process name and checks the filter before reading the status file (TryReadStatusFile) and enumerating threads. This avoids unnecessary I/O for non-matching processes when calling GetProcessesByName. The old Debug.Assert(processNameFilter is null) was correctly removed since filtering is now supported.

✅ Dead code removal is clean

All deleted code is verifiably dead:

• GetProcessIdFromHandle (Windows + Unix) → single call site replaced with SafeProcessHandle.ProcessId (Process.cs:864)
• GetProcessIds() on Linux/SunOS → only BSD called GetProcessIds(); Linux/SunOS used EnumerateProcessIds() directly
• ProcessManager.cs shared IsRemoteMachine wrapper → logic consolidated into platform-specific HandleRemoteMachineSupport and IsRemoteMachine
• NtProcessManager.GetProcessIds(string, bool) → callers now use GetProcessIds(machineName, isRemoteMachine) on ProcessManager which extracts IDs from process infos

✅ NtProcessInfoHelper return type change is correct

Changing from ProcessInfo[] to Dictionary<int, ProcessInfo> avoids a CopyTo allocation. The TryGetValue in GetProcessInfo (local path) is functionally equivalent to the old Length == 1 check, since the processIdFilter ensures at most one matching entry per pid in the dictionary.

✅ Argument validation preserved

ArgumentException.ThrowIfNullOrEmpty(machineName) is now in HandleRemoteMachineSupport on both Windows and Unix. Previously it was in the shared ProcessManager.IsRemoteMachine wrapper. All public entry points with machineName call HandleRemoteMachineSupport, so validation is preserved.

💡 Minor style: braceless foreach body

In ProcessManager.Windows.cs:51-52, the foreach body is a single statement without braces:

foreach(KeyValuePair<int,ProcessInfo>entryinprocessInfos)builder.Add(entry.Value);

The latest commit in this PR (0159e64) specifically added braces to a for loop per reviewer feedback. Consistency would suggest braces here too. Very minor — this may have been explicitly accepted.

Generated by Code Review for issue #126338 · ◷

[jkotas]

In ProcessManager.Windows.cs:51-52, the foreach body is a single statement without braces:
foreach (KeyValuePair<int, ProcessInfo> entry in processInfos)
builder.Add(entry.Value);
The latest commit in this PR (https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fdotnet%2Fruntime%2Fcommit%2F0159e644f53613a497f70a453c4c4d6091ee5c76&data=05%7C02%7Cjkotas%40microsoft.com%7C596478d74d044791b75808de91bfbff0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C639108449913323022%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=b%2BGgCxoL86U3Epk%2BcLD4de4Tkx4nJg5I7hmepcOu66E%3D&reserved=0) specifically added braces to a for loop per reviewer feedback. Consistency would suggest braces here too. Very minor — this may have been explicitly accepted.

@copilot Address this feedback

[dotnet-policy-service]

Tagging subscribers to 'size-reduction': @eerhardt, @SamMonoRT
See info in area-owners.md if you want to be subscribed.

[Copilot]

A NativeAOT app doing only Process.Start("notepad.exe").WaitForExit() pulled in PerformanceCounterLib → PerformanceMonitor → RegistryKey.OpenRemoteBaseKey due to reachability chains through Process.ToString(), ProcessManager.OpenProcess, GetProcesses(), and GetProcessesByName(). This PR breaks all these chains using the lazily-initialized delegate technique and local-only entry points. It also restores the name-filter efficiency for GetProcessesByName on all platforms.

Win-x64 NAOT app that does Process.Start("notepad.exe").WaitForExit(); goes from 1550kB to 1210kB (-22%, 340kB code size reduction).

Contributes to #126332

[jkotas]

@copilot Can you measure how much code more code can be trimmed thanks to this change in a simple Process.Start("notepad.exe").WaitForExit() app on Windows?

[Copilot]

@copilot Can you measure how much code more code can be trimmed thanks to this change in a simple Process.Start("notepad.exe").WaitForExit() app on Windows?

I'm unable to produce actual size numbers from this Linux sandbox — NativeAOT Windows builds require a Windows environment with the ILCompiler toolchain. The change only affects Windows-specific code (ProcessManager.Windows.cs), so a Linux NativeAOT equivalent wouldn't reflect the real impact.

Based on static analysis, the code eliminated from a trimmed Process.Start("notepad.exe").WaitForExit() app is:

In ProcessManager.Windows.cs:

• NtProcessManager.GetProcessInfos(string machineName, bool isRemoteMachine) (~20 lines)
• NtProcessManager.GetProcessInfos(PerformanceCounterLib library) (~30 lines)
• NtProcessManager.GetProcessInfos(PerformanceCounterLib library, int processIndex, int threadIndex, ReadOnlySpan<byte>) (~90 lines)

The entire PerformanceCounterLib.cs file (~286 lines), including:

• PerformanceCounterLib class (all methods + static dictionary)
• PerformanceMonitor nested class (including RegistryKey.OpenRemoteBaseKey)

Transitively from RegistryKey.OpenRemoteBaseKey: Remote registry infrastructure (from Microsoft.Win32.Registry).

To get exact byte deltas, you'd need to publish the test app as NativeAOT on Windows (e.g. dotnet publish -r win-x64 -p:PublishAot=true) against both the base branch and this branch and compare the .exe sizes and/or ILLink .map files. If you have a Windows machine with the NativeAOT toolchain, happy to provide the test project.

[jkotas]

💡 Minor: Extra blank lines
Two extra blank lines were introduced:

Process.Windows.cs line ~509: double blank line between GetStandardOutputEncoding and StartCore
Process.cs line ~871: double blank line between the remote-machine check and the exited check in EnsureState
These are cosmetic and not blocking, but worth cleaning up in a follow-up if convenient.

💡 Minor: Inconsistent null-check style
NtProcessInfoHelper.GetProcessInfos (line 1069) uses processNameFilter == null while other locations in the same PR use processNameFilter is null (e.g., NtProcessManager.GetProcessInfos line 500, ProcessManager.BSD.cs line 19). Per dotnet/runtime conventions, is null is preferred. This is a pre-existing inconsistency that the PR partially cleans up but doesn't fully resolve. Non-blocking.

@copilot

[Copilot]

💡 Minor: Extra blank lines
Two extra blank lines were introduced:

...

Done in 37d8c56. Removed both extra blank lines (Process.Windows.cs between GetStandardOutputEncoding and StartCore, and Process.cs in EnsureState) and changed processNameFilter == null to processNameFilter is null in NtProcessInfoHelper.GetProcessInfos.

[adamsitnik]

LGTM, thank you @jkotas !

[github-actions]

🤖 Copilot Code Review — PR #126338

Note

This review was AI/Copilot-generated.

Holistic Assessment

Motivation: Justified. Improving trimability of System.Diagnostics.Process by breaking the static reference chain to PerformanceCounterLib for apps that only use local-machine APIs (e.g., Process.Start) is a clear win for NativeAOT and trimmed deployments. The motivation is evident from the code itself and confirmed by the iterative refinement with @jkotas over ~50 commits.

Approach: Sound. Using a lazily-initialized delegate (s_getRemoteProcessInfos) to defer loading remote machine code is the right pattern for trimability. Splitting local-only overloads (GetProcesses(), GetProcessesByName(string?), GetProcessById(int)) from their remote-supporting counterparts so they don't pull in PerformanceCounterLib is clean and well-executed. Moving platform-specific GetProcessesByName into a unified implementation in Process.cs reduces duplication. The shift to ref ArrayBuilder<ProcessInfo> eliminates intermediate array allocations.

Summary: ✅ LGTM. The refactoring is correct, well-motivated, and preserves existing behavior across all platforms. No blocking issues found. All code paths were verified for correctness, argument validation preservation, and behavioral equivalence.

---

Detailed Findings

✅ Delegate safety — s_getRemoteProcessInfos null-safety verified

The delegate is only dereferenced with ! in paths where isRemoteMachine == true. All callers that pass isRemoteMachine=true receive that value from HandleRemoteMachineSupport(machineName), which sets the delegate via s_getRemoteProcessInfos ??= NtProcessManager.GetProcessInfos before returning true. The ??= race is benign — multiple threads may race to assign, but they all write the same static method group value.

Verified at: ProcessManager.Windows.cs:38 (assignment), lines 114, 128, 181 (dereferences).

✅ Behavioral equivalence across platforms

The unified GetProcessesByName(string?, string) in Process.cs correctly delegates to HandleRemoteMachineSupport + GetProcessInfos on all platforms:

• Windows: Remote → s_getRemoteProcessInfos (PerformanceCounterLib path); Local → NtProcessInfoHelper (NtQuerySystemInformation path). Same as before.
• Linux/BSD/OSX/SunOS: HandleRemoteMachineSupport throws PlatformNotSupportedException for remote; local calls GetProcessInfos(ref builder, processNameFilter). Same as before, with the ThrowIfRemoteMachine validation now in HandleRemoteMachineSupport.
• iOS: Old code threw PlatformNotSupportedException unconditionally. New code validates machineName first (via HandleRemoteMachineSupport), then throws PNSE from GetProcessInfos. Minor improvement — argument validation now happens before PNSE.

Process name normalization (null/empty → null meaning "no filter") is consistent at all call sites.

✅ Linux early filtering optimization

ProcessManager.Linux.CreateProcessInfo(pid, processNameFilter) now resolves the process name and checks the filter before reading the status file (TryReadStatusFile) and enumerating threads. This avoids unnecessary I/O for non-matching processes when calling GetProcessesByName. The old Debug.Assert(processNameFilter is null) was correctly removed since filtering is now supported.

✅ Dead code removal is clean

All deleted code is verifiably dead:

• GetProcessIdFromHandle (Windows + Unix) → single call site replaced with SafeProcessHandle.ProcessId (Process.cs:864)
• GetProcessIds() on Linux/SunOS → only BSD called GetProcessIds(); Linux/SunOS used EnumerateProcessIds() directly
• ProcessManager.cs shared IsRemoteMachine wrapper → logic consolidated into platform-specific HandleRemoteMachineSupport and IsRemoteMachine
• NtProcessManager.GetProcessIds(string, bool) → callers now use GetProcessIds(machineName, isRemoteMachine) on ProcessManager which extracts IDs from process infos

✅ NtProcessInfoHelper return type change is correct

Changing from ProcessInfo[] to Dictionary<int, ProcessInfo> avoids a CopyTo allocation. The TryGetValue in GetProcessInfo (local path) is functionally equivalent to the old Length == 1 check, since the processIdFilter ensures at most one matching entry per pid in the dictionary.

✅ Argument validation preserved

ArgumentException.ThrowIfNullOrEmpty(machineName) is now in HandleRemoteMachineSupport on both Windows and Unix. Previously it was in the shared ProcessManager.IsRemoteMachine wrapper. All public entry points with machineName call HandleRemoteMachineSupport, so validation is preserved.

💡 Minor style: braceless foreach body

In ProcessManager.Windows.cs:51-52, the foreach body is a single statement without braces:

foreach(KeyValuePair<int,ProcessInfo>entryinprocessInfos)builder.Add(entry.Value);

The latest commit in this PR (0159e64) specifically added braces to a for loop per reviewer feedback. Consistency would suggest braces here too. Very minor — this may have been explicitly accepted.

Generated by Code Review for issue #126338 · ◷

[jkotas]

In ProcessManager.Windows.cs:51-52, the foreach body is a single statement without braces:
foreach (KeyValuePair<int, ProcessInfo> entry in processInfos)
builder.Add(entry.Value);
The latest commit in this PR (https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fdotnet%2Fruntime%2Fcommit%2F0159e644f53613a497f70a453c4c4d6091ee5c76&data=05%7C02%7Cjkotas%40microsoft.com%7C596478d74d044791b75808de91bfbff0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C639108449913323022%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=b%2BGgCxoL86U3Epk%2BcLD4de4Tkx4nJg5I7hmepcOu66E%3D&reserved=0) specifically added braces to a for loop per reviewer feedback. Consistency would suggest braces here too. Very minor — this may have been explicitly accepted.

@copilot Address this feedback

[dotnet-policy-service]

Tagging subscribers to 'size-reduction': @eerhardt, @SamMonoRT
See info in area-owners.md if you want to be subscribed.

[Copilot]

A NativeAOT app doing only Process.Start("notepad.exe").WaitForExit() pulled in PerformanceCounterLib → PerformanceMonitor → RegistryKey.OpenRemoteBaseKey due to reachability chains through Process.ToString(), ProcessManager.OpenProcess, GetProcesses(), and GetProcessesByName(). This PR breaks all these chains using the lazily-initialized delegate technique and local-only entry points. It also restores the name-filter efficiency for GetProcessesByName on all platforms.

Win-x64 NAOT app that does Process.Start("notepad.exe").WaitForExit(); goes from 1550kB to 1210kB (-22%, 340kB code size reduction).

Contributes to #126332

[jkotas]

@copilot Can you measure how much code more code can be trimmed thanks to this change in a simple Process.Start("notepad.exe").WaitForExit() app on Windows?

[Copilot]

@copilot Can you measure how much code more code can be trimmed thanks to this change in a simple Process.Start("notepad.exe").WaitForExit() app on Windows?

I'm unable to produce actual size numbers from this Linux sandbox — NativeAOT Windows builds require a Windows environment with the ILCompiler toolchain. The change only affects Windows-specific code (ProcessManager.Windows.cs), so a Linux NativeAOT equivalent wouldn't reflect the real impact.

Based on static analysis, the code eliminated from a trimmed Process.Start("notepad.exe").WaitForExit() app is:

In ProcessManager.Windows.cs:

• NtProcessManager.GetProcessInfos(string machineName, bool isRemoteMachine) (~20 lines)
• NtProcessManager.GetProcessInfos(PerformanceCounterLib library) (~30 lines)
• NtProcessManager.GetProcessInfos(PerformanceCounterLib library, int processIndex, int threadIndex, ReadOnlySpan<byte>) (~90 lines)

The entire PerformanceCounterLib.cs file (~286 lines), including:

• PerformanceCounterLib class (all methods + static dictionary)
• PerformanceMonitor nested class (including RegistryKey.OpenRemoteBaseKey)

Transitively from RegistryKey.OpenRemoteBaseKey: Remote registry infrastructure (from Microsoft.Win32.Registry).

To get exact byte deltas, you'd need to publish the test app as NativeAOT on Windows (e.g. dotnet publish -r win-x64 -p:PublishAot=true) against both the base branch and this branch and compare the .exe sizes and/or ILLink .map files. If you have a Windows machine with the NativeAOT toolchain, happy to provide the test project.

[jkotas]

💡 Minor: Extra blank lines
Two extra blank lines were introduced:

Process.Windows.cs line ~509: double blank line between GetStandardOutputEncoding and StartCore
Process.cs line ~871: double blank line between the remote-machine check and the exited check in EnsureState
These are cosmetic and not blocking, but worth cleaning up in a follow-up if convenient.

💡 Minor: Inconsistent null-check style
NtProcessInfoHelper.GetProcessInfos (line 1069) uses processNameFilter == null while other locations in the same PR use processNameFilter is null (e.g., NtProcessManager.GetProcessInfos line 500, ProcessManager.BSD.cs line 19). Per dotnet/runtime conventions, is null is preferred. This is a pre-existing inconsistency that the PR partially cleans up but doesn't fully resolve. Non-blocking.

@copilot

[Copilot]

💡 Minor: Extra blank lines
Two extra blank lines were introduced:

...

Done in 37d8c56. Removed both extra blank lines (Process.Windows.cs between GetStandardOutputEncoding and StartCore, and Process.cs in EnsureState) and changed processNameFilter == null to processNameFilter is null in NtProcessInfoHelper.GetProcessInfos.

[adamsitnik]

LGTM, thank you @jkotas !

[github-actions]

🤖 Copilot Code Review — PR #126338

Note

This review was AI/Copilot-generated.

Holistic Assessment

Motivation: Justified. Improving trimability of System.Diagnostics.Process by breaking the static reference chain to PerformanceCounterLib for apps that only use local-machine APIs (e.g., Process.Start) is a clear win for NativeAOT and trimmed deployments. The motivation is evident from the code itself and confirmed by the iterative refinement with @jkotas over ~50 commits.

Approach: Sound. Using a lazily-initialized delegate (s_getRemoteProcessInfos) to defer loading remote machine code is the right pattern for trimability. Splitting local-only overloads (GetProcesses(), GetProcessesByName(string?), GetProcessById(int)) from their remote-supporting counterparts so they don't pull in PerformanceCounterLib is clean and well-executed. Moving platform-specific GetProcessesByName into a unified implementation in Process.cs reduces duplication. The shift to ref ArrayBuilder<ProcessInfo> eliminates intermediate array allocations.

Summary: ✅ LGTM. The refactoring is correct, well-motivated, and preserves existing behavior across all platforms. No blocking issues found. All code paths were verified for correctness, argument validation preservation, and behavioral equivalence.

---

Detailed Findings

✅ Delegate safety — s_getRemoteProcessInfos null-safety verified

The delegate is only dereferenced with ! in paths where isRemoteMachine == true. All callers that pass isRemoteMachine=true receive that value from HandleRemoteMachineSupport(machineName), which sets the delegate via s_getRemoteProcessInfos ??= NtProcessManager.GetProcessInfos before returning true. The ??= race is benign — multiple threads may race to assign, but they all write the same static method group value.

Verified at: ProcessManager.Windows.cs:38 (assignment), lines 114, 128, 181 (dereferences).

✅ Behavioral equivalence across platforms

The unified GetProcessesByName(string?, string) in Process.cs correctly delegates to HandleRemoteMachineSupport + GetProcessInfos on all platforms:

• Windows: Remote → s_getRemoteProcessInfos (PerformanceCounterLib path); Local → NtProcessInfoHelper (NtQuerySystemInformation path). Same as before.
• Linux/BSD/OSX/SunOS: HandleRemoteMachineSupport throws PlatformNotSupportedException for remote; local calls GetProcessInfos(ref builder, processNameFilter). Same as before, with the ThrowIfRemoteMachine validation now in HandleRemoteMachineSupport.
• iOS: Old code threw PlatformNotSupportedException unconditionally. New code validates machineName first (via HandleRemoteMachineSupport), then throws PNSE from GetProcessInfos. Minor improvement — argument validation now happens before PNSE.

Process name normalization (null/empty → null meaning "no filter") is consistent at all call sites.

✅ Linux early filtering optimization

ProcessManager.Linux.CreateProcessInfo(pid, processNameFilter) now resolves the process name and checks the filter before reading the status file (TryReadStatusFile) and enumerating threads. This avoids unnecessary I/O for non-matching processes when calling GetProcessesByName. The old Debug.Assert(processNameFilter is null) was correctly removed since filtering is now supported.

✅ Dead code removal is clean

All deleted code is verifiably dead:

• GetProcessIdFromHandle (Windows + Unix) → single call site replaced with SafeProcessHandle.ProcessId (Process.cs:864)
• GetProcessIds() on Linux/SunOS → only BSD called GetProcessIds(); Linux/SunOS used EnumerateProcessIds() directly
• ProcessManager.cs shared IsRemoteMachine wrapper → logic consolidated into platform-specific HandleRemoteMachineSupport and IsRemoteMachine
• NtProcessManager.GetProcessIds(string, bool) → callers now use GetProcessIds(machineName, isRemoteMachine) on ProcessManager which extracts IDs from process infos

✅ NtProcessInfoHelper return type change is correct

Changing from ProcessInfo[] to Dictionary<int, ProcessInfo> avoids a CopyTo allocation. The TryGetValue in GetProcessInfo (local path) is functionally equivalent to the old Length == 1 check, since the processIdFilter ensures at most one matching entry per pid in the dictionary.

✅ Argument validation preserved

ArgumentException.ThrowIfNullOrEmpty(machineName) is now in HandleRemoteMachineSupport on both Windows and Unix. Previously it was in the shared ProcessManager.IsRemoteMachine wrapper. All public entry points with machineName call HandleRemoteMachineSupport, so validation is preserved.

💡 Minor style: braceless foreach body

In ProcessManager.Windows.cs:51-52, the foreach body is a single statement without braces:

foreach(KeyValuePair<int,ProcessInfo>entryinprocessInfos)builder.Add(entry.Value);

The latest commit in this PR (0159e64) specifically added braces to a for loop per reviewer feedback. Consistency would suggest braces here too. Very minor — this may have been explicitly accepted.

Generated by Code Review for issue #126338 · ◷

[jkotas]

In ProcessManager.Windows.cs:51-52, the foreach body is a single statement without braces:
foreach (KeyValuePair<int, ProcessInfo> entry in processInfos)
builder.Add(entry.Value);
The latest commit in this PR (https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fdotnet%2Fruntime%2Fcommit%2F0159e644f53613a497f70a453c4c4d6091ee5c76&data=05%7C02%7Cjkotas%40microsoft.com%7C596478d74d044791b75808de91bfbff0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C639108449913323022%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=b%2BGgCxoL86U3Epk%2BcLD4de4Tkx4nJg5I7hmepcOu66E%3D&reserved=0) specifically added braces to a for loop per reviewer feedback. Consistency would suggest braces here too. Very minor — this may have been explicitly accepted.

@copilot Address this feedback

[dotnet-policy-service]

Tagging subscribers to 'size-reduction': @eerhardt, @SamMonoRT
See info in area-owners.md if you want to be subscribed.

[Copilot]

A NativeAOT app doing only Process.Start("notepad.exe").WaitForExit() pulled in PerformanceCounterLib → PerformanceMonitor → RegistryKey.OpenRemoteBaseKey due to reachability chains through Process.ToString(), ProcessManager.OpenProcess, GetProcesses(), and GetProcessesByName(). This PR breaks all these chains using the lazily-initialized delegate technique and local-only entry points. It also restores the name-filter efficiency for GetProcessesByName on all platforms.

Win-x64 NAOT app that does Process.Start("notepad.exe").WaitForExit(); goes from 1550kB to 1210kB (-22%, 340kB code size reduction).

Contributes to #126332

[jkotas]

@copilot Can you measure how much code more code can be trimmed thanks to this change in a simple Process.Start("notepad.exe").WaitForExit() app on Windows?

[Copilot]

@copilot Can you measure how much code more code can be trimmed thanks to this change in a simple Process.Start("notepad.exe").WaitForExit() app on Windows?

I'm unable to produce actual size numbers from this Linux sandbox — NativeAOT Windows builds require a Windows environment with the ILCompiler toolchain. The change only affects Windows-specific code (ProcessManager.Windows.cs), so a Linux NativeAOT equivalent wouldn't reflect the real impact.

Based on static analysis, the code eliminated from a trimmed Process.Start("notepad.exe").WaitForExit() app is:

In ProcessManager.Windows.cs:

• NtProcessManager.GetProcessInfos(string machineName, bool isRemoteMachine) (~20 lines)
• NtProcessManager.GetProcessInfos(PerformanceCounterLib library) (~30 lines)
• NtProcessManager.GetProcessInfos(PerformanceCounterLib library, int processIndex, int threadIndex, ReadOnlySpan<byte>) (~90 lines)

The entire PerformanceCounterLib.cs file (~286 lines), including:

• PerformanceCounterLib class (all methods + static dictionary)
• PerformanceMonitor nested class (including RegistryKey.OpenRemoteBaseKey)

Transitively from RegistryKey.OpenRemoteBaseKey: Remote registry infrastructure (from Microsoft.Win32.Registry).

To get exact byte deltas, you'd need to publish the test app as NativeAOT on Windows (e.g. dotnet publish -r win-x64 -p:PublishAot=true) against both the base branch and this branch and compare the .exe sizes and/or ILLink .map files. If you have a Windows machine with the NativeAOT toolchain, happy to provide the test project.

[jkotas]

💡 Minor: Extra blank lines
Two extra blank lines were introduced:

Process.Windows.cs line ~509: double blank line between GetStandardOutputEncoding and StartCore
Process.cs line ~871: double blank line between the remote-machine check and the exited check in EnsureState
These are cosmetic and not blocking, but worth cleaning up in a follow-up if convenient.

💡 Minor: Inconsistent null-check style
NtProcessInfoHelper.GetProcessInfos (line 1069) uses processNameFilter == null while other locations in the same PR use processNameFilter is null (e.g., NtProcessManager.GetProcessInfos line 500, ProcessManager.BSD.cs line 19). Per dotnet/runtime conventions, is null is preferred. This is a pre-existing inconsistency that the PR partially cleans up but doesn't fully resolve. Non-blocking.

@copilot

[Copilot]

💡 Minor: Extra blank lines
Two extra blank lines were introduced:

...

Done in 37d8c56. Removed both extra blank lines (Process.Windows.cs between GetStandardOutputEncoding and StartCore, and Process.cs in EnsureState) and changed processNameFilter == null to processNameFilter is null in NtProcessInfoHelper.GetProcessInfos.

[adamsitnik]

LGTM, thank you @jkotas !

[github-actions]

🤖 Copilot Code Review — PR #126338

Note

This review was AI/Copilot-generated.

Holistic Assessment

Motivation: Justified. Improving trimability of System.Diagnostics.Process by breaking the static reference chain to PerformanceCounterLib for apps that only use local-machine APIs (e.g., Process.Start) is a clear win for NativeAOT and trimmed deployments. The motivation is evident from the code itself and confirmed by the iterative refinement with @jkotas over ~50 commits.

Approach: Sound. Using a lazily-initialized delegate (s_getRemoteProcessInfos) to defer loading remote machine code is the right pattern for trimability. Splitting local-only overloads (GetProcesses(), GetProcessesByName(string?), GetProcessById(int)) from their remote-supporting counterparts so they don't pull in PerformanceCounterLib is clean and well-executed. Moving platform-specific GetProcessesByName into a unified implementation in Process.cs reduces duplication. The shift to ref ArrayBuilder<ProcessInfo> eliminates intermediate array allocations.

Summary: ✅ LGTM. The refactoring is correct, well-motivated, and preserves existing behavior across all platforms. No blocking issues found. All code paths were verified for correctness, argument validation preservation, and behavioral equivalence.

---

Detailed Findings

✅ Delegate safety — s_getRemoteProcessInfos null-safety verified

The delegate is only dereferenced with ! in paths where isRemoteMachine == true. All callers that pass isRemoteMachine=true receive that value from HandleRemoteMachineSupport(machineName), which sets the delegate via s_getRemoteProcessInfos ??= NtProcessManager.GetProcessInfos before returning true. The ??= race is benign — multiple threads may race to assign, but they all write the same static method group value.

Verified at: ProcessManager.Windows.cs:38 (assignment), lines 114, 128, 181 (dereferences).

✅ Behavioral equivalence across platforms

The unified GetProcessesByName(string?, string) in Process.cs correctly delegates to HandleRemoteMachineSupport + GetProcessInfos on all platforms:

• Windows: Remote → s_getRemoteProcessInfos (PerformanceCounterLib path); Local → NtProcessInfoHelper (NtQuerySystemInformation path). Same as before.
• Linux/BSD/OSX/SunOS: HandleRemoteMachineSupport throws PlatformNotSupportedException for remote; local calls GetProcessInfos(ref builder, processNameFilter). Same as before, with the ThrowIfRemoteMachine validation now in HandleRemoteMachineSupport.
• iOS: Old code threw PlatformNotSupportedException unconditionally. New code validates machineName first (via HandleRemoteMachineSupport), then throws PNSE from GetProcessInfos. Minor improvement — argument validation now happens before PNSE.

Process name normalization (null/empty → null meaning "no filter") is consistent at all call sites.

✅ Linux early filtering optimization

ProcessManager.Linux.CreateProcessInfo(pid, processNameFilter) now resolves the process name and checks the filter before reading the status file (TryReadStatusFile) and enumerating threads. This avoids unnecessary I/O for non-matching processes when calling GetProcessesByName. The old Debug.Assert(processNameFilter is null) was correctly removed since filtering is now supported.

✅ Dead code removal is clean

All deleted code is verifiably dead:

• GetProcessIdFromHandle (Windows + Unix) → single call site replaced with SafeProcessHandle.ProcessId (Process.cs:864)
• GetProcessIds() on Linux/SunOS → only BSD called GetProcessIds(); Linux/SunOS used EnumerateProcessIds() directly
• ProcessManager.cs shared IsRemoteMachine wrapper → logic consolidated into platform-specific HandleRemoteMachineSupport and IsRemoteMachine
• NtProcessManager.GetProcessIds(string, bool) → callers now use GetProcessIds(machineName, isRemoteMachine) on ProcessManager which extracts IDs from process infos

✅ NtProcessInfoHelper return type change is correct

Changing from ProcessInfo[] to Dictionary<int, ProcessInfo> avoids a CopyTo allocation. The TryGetValue in GetProcessInfo (local path) is functionally equivalent to the old Length == 1 check, since the processIdFilter ensures at most one matching entry per pid in the dictionary.

✅ Argument validation preserved

ArgumentException.ThrowIfNullOrEmpty(machineName) is now in HandleRemoteMachineSupport on both Windows and Unix. Previously it was in the shared ProcessManager.IsRemoteMachine wrapper. All public entry points with machineName call HandleRemoteMachineSupport, so validation is preserved.

💡 Minor style: braceless foreach body

In ProcessManager.Windows.cs:51-52, the foreach body is a single statement without braces:

foreach(KeyValuePair<int,ProcessInfo>entryinprocessInfos)builder.Add(entry.Value);

The latest commit in this PR (0159e64) specifically added braces to a for loop per reviewer feedback. Consistency would suggest braces here too. Very minor — this may have been explicitly accepted.

Generated by Code Review for issue #126338 · ◷

[jkotas]

In ProcessManager.Windows.cs:51-52, the foreach body is a single statement without braces:
foreach (KeyValuePair<int, ProcessInfo> entry in processInfos)
builder.Add(entry.Value);
The latest commit in this PR (https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fdotnet%2Fruntime%2Fcommit%2F0159e644f53613a497f70a453c4c4d6091ee5c76&data=05%7C02%7Cjkotas%40microsoft.com%7C596478d74d044791b75808de91bfbff0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C639108449913323022%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=b%2BGgCxoL86U3Epk%2BcLD4de4Tkx4nJg5I7hmepcOu66E%3D&reserved=0) specifically added braces to a for loop per reviewer feedback. Consistency would suggest braces here too. Very minor — this may have been explicitly accepted.

@copilot Address this feedback

[dotnet-policy-service]

Tagging subscribers to 'size-reduction': @eerhardt, @SamMonoRT
See info in area-owners.md if you want to be subscribed.

[Copilot]

A NativeAOT app doing only Process.Start("notepad.exe").WaitForExit() pulled in PerformanceCounterLib → PerformanceMonitor → RegistryKey.OpenRemoteBaseKey due to reachability chains through Process.ToString(), ProcessManager.OpenProcess, GetProcesses(), and GetProcessesByName(). This PR breaks all these chains using the lazily-initialized delegate technique and local-only entry points. It also restores the name-filter efficiency for GetProcessesByName on all platforms.

Win-x64 NAOT app that does Process.Start("notepad.exe").WaitForExit(); goes from 1550kB to 1210kB (-22%, 340kB code size reduction).

Contributes to #126332

[jkotas]

@copilot Can you measure how much code more code can be trimmed thanks to this change in a simple Process.Start("notepad.exe").WaitForExit() app on Windows?

[Copilot]

@copilot Can you measure how much code more code can be trimmed thanks to this change in a simple Process.Start("notepad.exe").WaitForExit() app on Windows?

I'm unable to produce actual size numbers from this Linux sandbox — NativeAOT Windows builds require a Windows environment with the ILCompiler toolchain. The change only affects Windows-specific code (ProcessManager.Windows.cs), so a Linux NativeAOT equivalent wouldn't reflect the real impact.

Based on static analysis, the code eliminated from a trimmed Process.Start("notepad.exe").WaitForExit() app is:

In ProcessManager.Windows.cs:

• NtProcessManager.GetProcessInfos(string machineName, bool isRemoteMachine) (~20 lines)
• NtProcessManager.GetProcessInfos(PerformanceCounterLib library) (~30 lines)
• NtProcessManager.GetProcessInfos(PerformanceCounterLib library, int processIndex, int threadIndex, ReadOnlySpan<byte>) (~90 lines)

The entire PerformanceCounterLib.cs file (~286 lines), including:

• PerformanceCounterLib class (all methods + static dictionary)
• PerformanceMonitor nested class (including RegistryKey.OpenRemoteBaseKey)

Transitively from RegistryKey.OpenRemoteBaseKey: Remote registry infrastructure (from Microsoft.Win32.Registry).

To get exact byte deltas, you'd need to publish the test app as NativeAOT on Windows (e.g. dotnet publish -r win-x64 -p:PublishAot=true) against both the base branch and this branch and compare the .exe sizes and/or ILLink .map files. If you have a Windows machine with the NativeAOT toolchain, happy to provide the test project.

[jkotas]

💡 Minor: Extra blank lines
Two extra blank lines were introduced:

Process.Windows.cs line ~509: double blank line between GetStandardOutputEncoding and StartCore
Process.cs line ~871: double blank line between the remote-machine check and the exited check in EnsureState
These are cosmetic and not blocking, but worth cleaning up in a follow-up if convenient.

💡 Minor: Inconsistent null-check style
NtProcessInfoHelper.GetProcessInfos (line 1069) uses processNameFilter == null while other locations in the same PR use processNameFilter is null (e.g., NtProcessManager.GetProcessInfos line 500, ProcessManager.BSD.cs line 19). Per dotnet/runtime conventions, is null is preferred. This is a pre-existing inconsistency that the PR partially cleans up but doesn't fully resolve. Non-blocking.

@copilot

[Copilot]

💡 Minor: Extra blank lines
Two extra blank lines were introduced:

...

Done in 37d8c56. Removed both extra blank lines (Process.Windows.cs between GetStandardOutputEncoding and StartCore, and Process.cs in EnsureState) and changed processNameFilter == null to processNameFilter is null in NtProcessInfoHelper.GetProcessInfos.

[adamsitnik]

LGTM, thank you @jkotas !

[github-actions]

🤖 Copilot Code Review — PR #126338

Note

This review was AI/Copilot-generated.

Holistic Assessment

Motivation: Justified. Improving trimability of System.Diagnostics.Process by breaking the static reference chain to PerformanceCounterLib for apps that only use local-machine APIs (e.g., Process.Start) is a clear win for NativeAOT and trimmed deployments. The motivation is evident from the code itself and confirmed by the iterative refinement with @jkotas over ~50 commits.

Approach: Sound. Using a lazily-initialized delegate (s_getRemoteProcessInfos) to defer loading remote machine code is the right pattern for trimability. Splitting local-only overloads (GetProcesses(), GetProcessesByName(string?), GetProcessById(int)) from their remote-supporting counterparts so they don't pull in PerformanceCounterLib is clean and well-executed. Moving platform-specific GetProcessesByName into a unified implementation in Process.cs reduces duplication. The shift to ref ArrayBuilder<ProcessInfo> eliminates intermediate array allocations.

Summary: ✅ LGTM. The refactoring is correct, well-motivated, and preserves existing behavior across all platforms. No blocking issues found. All code paths were verified for correctness, argument validation preservation, and behavioral equivalence.

---

Detailed Findings

✅ Delegate safety — s_getRemoteProcessInfos null-safety verified

The delegate is only dereferenced with ! in paths where isRemoteMachine == true. All callers that pass isRemoteMachine=true receive that value from HandleRemoteMachineSupport(machineName), which sets the delegate via s_getRemoteProcessInfos ??= NtProcessManager.GetProcessInfos before returning true. The ??= race is benign — multiple threads may race to assign, but they all write the same static method group value.

Verified at: ProcessManager.Windows.cs:38 (assignment), lines 114, 128, 181 (dereferences).

✅ Behavioral equivalence across platforms

The unified GetProcessesByName(string?, string) in Process.cs correctly delegates to HandleRemoteMachineSupport + GetProcessInfos on all platforms:

• Windows: Remote → s_getRemoteProcessInfos (PerformanceCounterLib path); Local → NtProcessInfoHelper (NtQuerySystemInformation path). Same as before.
• Linux/BSD/OSX/SunOS: HandleRemoteMachineSupport throws PlatformNotSupportedException for remote; local calls GetProcessInfos(ref builder, processNameFilter). Same as before, with the ThrowIfRemoteMachine validation now in HandleRemoteMachineSupport.
• iOS: Old code threw PlatformNotSupportedException unconditionally. New code validates machineName first (via HandleRemoteMachineSupport), then throws PNSE from GetProcessInfos. Minor improvement — argument validation now happens before PNSE.

Process name normalization (null/empty → null meaning "no filter") is consistent at all call sites.

✅ Linux early filtering optimization

ProcessManager.Linux.CreateProcessInfo(pid, processNameFilter) now resolves the process name and checks the filter before reading the status file (TryReadStatusFile) and enumerating threads. This avoids unnecessary I/O for non-matching processes when calling GetProcessesByName. The old Debug.Assert(processNameFilter is null) was correctly removed since filtering is now supported.

✅ Dead code removal is clean

All deleted code is verifiably dead:

• GetProcessIdFromHandle (Windows + Unix) → single call site replaced with SafeProcessHandle.ProcessId (Process.cs:864)
• GetProcessIds() on Linux/SunOS → only BSD called GetProcessIds(); Linux/SunOS used EnumerateProcessIds() directly
• ProcessManager.cs shared IsRemoteMachine wrapper → logic consolidated into platform-specific HandleRemoteMachineSupport and IsRemoteMachine
• NtProcessManager.GetProcessIds(string, bool) → callers now use GetProcessIds(machineName, isRemoteMachine) on ProcessManager which extracts IDs from process infos

✅ NtProcessInfoHelper return type change is correct

Changing from ProcessInfo[] to Dictionary<int, ProcessInfo> avoids a CopyTo allocation. The TryGetValue in GetProcessInfo (local path) is functionally equivalent to the old Length == 1 check, since the processIdFilter ensures at most one matching entry per pid in the dictionary.

✅ Argument validation preserved

ArgumentException.ThrowIfNullOrEmpty(machineName) is now in HandleRemoteMachineSupport on both Windows and Unix. Previously it was in the shared ProcessManager.IsRemoteMachine wrapper. All public entry points with machineName call HandleRemoteMachineSupport, so validation is preserved.

💡 Minor style: braceless foreach body

In ProcessManager.Windows.cs:51-52, the foreach body is a single statement without braces:

foreach(KeyValuePair<int,ProcessInfo>entryinprocessInfos)builder.Add(entry.Value);

The latest commit in this PR (0159e64) specifically added braces to a for loop per reviewer feedback. Consistency would suggest braces here too. Very minor — this may have been explicitly accepted.

Generated by Code Review for issue #126338 · ◷

[jkotas]

In ProcessManager.Windows.cs:51-52, the foreach body is a single statement without braces:
foreach (KeyValuePair<int, ProcessInfo> entry in processInfos)
builder.Add(entry.Value);
The latest commit in this PR (https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fdotnet%2Fruntime%2Fcommit%2F0159e644f53613a497f70a453c4c4d6091ee5c76&data=05%7C02%7Cjkotas%40microsoft.com%7C596478d74d044791b75808de91bfbff0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C639108449913323022%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=b%2BGgCxoL86U3Epk%2BcLD4de4Tkx4nJg5I7hmepcOu66E%3D&reserved=0) specifically added braces to a for loop per reviewer feedback. Consistency would suggest braces here too. Very minor — this may have been explicitly accepted.

@copilot Address this feedback

[dotnet-policy-service]

Tagging subscribers to 'size-reduction': @eerhardt, @SamMonoRT
See info in area-owners.md if you want to be subscribed.