Reduce Unix Process.Start path resolution allocations

[Copilot]

• Path resolution
  • Move FindProgramInPath into the Unix implementation.
  • Build executable candidates with ValueStringBuilder; materialize strings only for successful matches.
  • Pass builder spans directly to executable checks via span-based UTF-8 interop.

Benchmark results from EgorBot/Benchmarks#551:

BenchmarkDotNet v0.16.0-preview.1, macOS Sequoia 15.4.1 (24E263) [Darwin 24.4.0]
Apple M4, 1 CPU, 10 logical and 10 physical cores
Memory: 16 GB Total, 4.36 GB Available
.NET SDK 11.0.100-rc.2.26426.116
[Host] : .NET 11.0.0 (11.0.0-rc.2.26426.116, 11.0.26.42716), Arm64 RyuJIT armv8.0-a

Method	Toolchain	Mean	Error	Ratio	Allocated	Alloc Ratio
Run	main	1,256.1 μs	2.72 μs	1.00	3.06 KB	2.43
Run	PR #132780	1,255.7 μs	2.76 μs	1.00	1.26 KB	1.00
Start	main	115.9 μs	2.56 μs	0.99	3.32 KB	2.19
Start	PR #132780	117.3 μs	3.29 μs	1.00	1.52 KB	1.00

BenchmarkDotNet v0.16.0-preview.1, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
AMD EPYC 9V45 2.60GHz, 1 CPU, 8 logical and 4 physical cores
Memory: 31.34 GB Total, 10.43 GB Available
.NET SDK 11.0.100-rc.2.26426.116
[Host] : .NET 11.0.0 (11.0.0-rc.2.26426.116, 11.0.26.42716), X64 RyuJIT x86-64-v4

Method	Toolchain	Mean	Error	Ratio	Allocated	Alloc Ratio
Run	main	633.4 μs	6.39 μs	1.00	2304 B	2.30
Run	PR #132780	631.3 μs	3.48 μs	1.00	1000 B	1.00
Start	main	156.9 μs	2.20 μs	0.93	2568 B	2.03
Start	PR #132780	169.4 μs	5.24 μs	1.00	1264 B	1.00

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[adamsitnik]

@copilot address my feedback

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[Copilot]

@copilot address my feedback

Addressed in a9eedc6: uses span-based Stat, centralizes path composition, and shares ResolvePath's builder with PATH lookup.

Note

AI/Copilot-generated response.

[adamsitnik]

@copilot it looks much better now. Please respond to my question

[adamsitnik]

@EgorBot -osx_arm64 -linux_x64

usingBenchmarkDotNet.Attributes;usingBenchmarkDotNet.Engines;usingBenchmarkDotNet.Running;usingMicrosoft.Win32.SafeHandles;usingSystem;usingSystem.Diagnostics;usingSystem.IO;usingSystem.Text;usingSystem.Threading.Tasks;BenchmarkSwitcher.FromAssembly(typeof(DefaultEnvVars).Assembly).Run(args);[MemoryDiagnoser]publicclassDefaultEnvVars{privatestaticreadonlySafeFileHandles_nullHandle=File.OpenNullHandle();privatestaticProcessStartInfos_startProcessStartInfo=newProcessStartInfo(){FileName="whoami",// exists on both Windows and Unix, and has very short outputStandardInputHandle=s_nullHandle,StandardOutputHandle=s_nullHandle,StandardErrorHandle=s_nullHandle,};privateProcess?_startedProcess;[Benchmark]publicvoidStart()=>_startedProcess=Process.Start(s_startProcessStartInfo)!;[IterationCleanup(Target=nameof(Start))]publicvoidCleanupStart(){if(_startedProcess!=null){_startedProcess.WaitForExit();_startedProcess.Dispose();_startedProcess=null;}}[Benchmark]publicintRun()=>Process.Run(s_startProcessStartInfo).ExitCode;[GlobalCleanup]publicvoidCleanup()=>s_nullHandle.Dispose();}

[adamsitnik]

@copilot please invoke the code-review skill and post your analysis/results as a comment on this PR

[Copilot]

@copilot please invoke the code-review skill and post your analysis/results as a comment on this PR

Holistic Review

Motivation: The code removes transient candidate-path strings from Unix executable lookup, a hot portion of Process.Start. The requested cross-architecture benchmark is the appropriate evidence for the claimed allocation reduction.

Approach: The change uses ValueStringBuilder with span-based UTF-8 marshalling, and materializes a string only after finding an executable. The ref-taking PATH helper correctly shares the builder with ResolvePath.

Summary: ⚠️ Needs Human Review. The code is sound after d7c184a, which resets the shared builder before PATH fallback; both independent reviewers found this same regression before the fix. The Process library Release build passes, but the requested benchmark results and focused test execution are not yet available for final performance and behavioral confirmation.

---

Detailed Findings

✅ Correctness — PATH fallback builder state

d7c184a clears the failed current-directory candidate before calling the ref-taking PATH helper and adds a debug assertion for its empty-builder contract. This fixes the prior behavior where the first PATH candidate was appended to the failed current-directory candidate.

✅ Allocation strategy and API surface

Candidate paths are composed in a stack-backed ValueStringBuilder; Access and Stat receive spans through the existing UTF-8 span marshaller. No public API surface was added. StringParser remains required by Linux procfs interop compiled into this project.

⚠️ Validation — benchmark and focused tests pending

The requested EgorBot benchmark has not produced results in the PR, so allocation improvement has not yet been measured on the requested Linux x64 and macOS arm64 configurations. The focused Process test project could not run locally because shared-framework/testhost prerequisites were unavailable; the Process library Release build did pass.

Note

AI/Copilot-generated review.

[adamsitnik]

@copilot please address the feedback where you have been tagged

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Unix.cs:373

• Same issue as above: using (pathBuilder) disposes a copy, which may not be the grown instance after ref mutations, and suppressing CS0728 hides a potential pooled-buffer leak. Prefer a using declaration on the local (or explicit try/finally) and avoid the extra nesting.

 string? path = Environment.ProcessPath;
ValueStringBuilder pathBuilder = new(stackalloc char[256]);
#pragma warning disable CS0728 // pathBuilder is intentionally mutated via ref; disposal still targets this local
using (pathBuilder)
{

[Copilot]

🟢 Approval recommended

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite

[adamsitnik]

/ba-g unrelated networking test failures

[Copilot]

• Path resolution
  • Move FindProgramInPath into the Unix implementation.
  • Build executable candidates with ValueStringBuilder; materialize strings only for successful matches.
  • Pass builder spans directly to executable checks via span-based UTF-8 interop.

Benchmark results from EgorBot/Benchmarks#551:

BenchmarkDotNet v0.16.0-preview.1, macOS Sequoia 15.4.1 (24E263) [Darwin 24.4.0]
Apple M4, 1 CPU, 10 logical and 10 physical cores
Memory: 16 GB Total, 4.36 GB Available
.NET SDK 11.0.100-rc.2.26426.116
[Host] : .NET 11.0.0 (11.0.0-rc.2.26426.116, 11.0.26.42716), Arm64 RyuJIT armv8.0-a

Method	Toolchain	Mean	Error	Ratio	Allocated	Alloc Ratio
Run	main	1,256.1 μs	2.72 μs	1.00	3.06 KB	2.43
Run	PR #132780	1,255.7 μs	2.76 μs	1.00	1.26 KB	1.00
Start	main	115.9 μs	2.56 μs	0.99	3.32 KB	2.19
Start	PR #132780	117.3 μs	3.29 μs	1.00	1.52 KB	1.00

BenchmarkDotNet v0.16.0-preview.1, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
AMD EPYC 9V45 2.60GHz, 1 CPU, 8 logical and 4 physical cores
Memory: 31.34 GB Total, 10.43 GB Available
.NET SDK 11.0.100-rc.2.26426.116
[Host] : .NET 11.0.0 (11.0.0-rc.2.26426.116, 11.0.26.42716), X64 RyuJIT x86-64-v4

Method	Toolchain	Mean	Error	Ratio	Allocated	Alloc Ratio
Run	main	633.4 μs	6.39 μs	1.00	2304 B	2.30
Run	PR #132780	631.3 μs	3.48 μs	1.00	1000 B	1.00
Start	main	156.9 μs	2.20 μs	0.93	2568 B	2.03
Start	PR #132780	169.4 μs	5.24 μs	1.00	1264 B	1.00

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[adamsitnik]

@copilot address my feedback

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[Copilot]

@copilot address my feedback

Addressed in a9eedc6: uses span-based Stat, centralizes path composition, and shares ResolvePath's builder with PATH lookup.

Note

AI/Copilot-generated response.

[adamsitnik]

@copilot it looks much better now. Please respond to my question

[adamsitnik]

@EgorBot -osx_arm64 -linux_x64

usingBenchmarkDotNet.Attributes;usingBenchmarkDotNet.Engines;usingBenchmarkDotNet.Running;usingMicrosoft.Win32.SafeHandles;usingSystem;usingSystem.Diagnostics;usingSystem.IO;usingSystem.Text;usingSystem.Threading.Tasks;BenchmarkSwitcher.FromAssembly(typeof(DefaultEnvVars).Assembly).Run(args);[MemoryDiagnoser]publicclassDefaultEnvVars{privatestaticreadonlySafeFileHandles_nullHandle=File.OpenNullHandle();privatestaticProcessStartInfos_startProcessStartInfo=newProcessStartInfo(){FileName="whoami",// exists on both Windows and Unix, and has very short outputStandardInputHandle=s_nullHandle,StandardOutputHandle=s_nullHandle,StandardErrorHandle=s_nullHandle,};privateProcess?_startedProcess;[Benchmark]publicvoidStart()=>_startedProcess=Process.Start(s_startProcessStartInfo)!;[IterationCleanup(Target=nameof(Start))]publicvoidCleanupStart(){if(_startedProcess!=null){_startedProcess.WaitForExit();_startedProcess.Dispose();_startedProcess=null;}}[Benchmark]publicintRun()=>Process.Run(s_startProcessStartInfo).ExitCode;[GlobalCleanup]publicvoidCleanup()=>s_nullHandle.Dispose();}

[adamsitnik]

@copilot please invoke the code-review skill and post your analysis/results as a comment on this PR

[Copilot]

@copilot please invoke the code-review skill and post your analysis/results as a comment on this PR

Holistic Review

Motivation: The code removes transient candidate-path strings from Unix executable lookup, a hot portion of Process.Start. The requested cross-architecture benchmark is the appropriate evidence for the claimed allocation reduction.

Approach: The change uses ValueStringBuilder with span-based UTF-8 marshalling, and materializes a string only after finding an executable. The ref-taking PATH helper correctly shares the builder with ResolvePath.

Summary: ⚠️ Needs Human Review. The code is sound after d7c184a, which resets the shared builder before PATH fallback; both independent reviewers found this same regression before the fix. The Process library Release build passes, but the requested benchmark results and focused test execution are not yet available for final performance and behavioral confirmation.

---

Detailed Findings

✅ Correctness — PATH fallback builder state

d7c184a clears the failed current-directory candidate before calling the ref-taking PATH helper and adds a debug assertion for its empty-builder contract. This fixes the prior behavior where the first PATH candidate was appended to the failed current-directory candidate.

✅ Allocation strategy and API surface

Candidate paths are composed in a stack-backed ValueStringBuilder; Access and Stat receive spans through the existing UTF-8 span marshaller. No public API surface was added. StringParser remains required by Linux procfs interop compiled into this project.

⚠️ Validation — benchmark and focused tests pending

The requested EgorBot benchmark has not produced results in the PR, so allocation improvement has not yet been measured on the requested Linux x64 and macOS arm64 configurations. The focused Process test project could not run locally because shared-framework/testhost prerequisites were unavailable; the Process library Release build did pass.

Note

AI/Copilot-generated review.

[adamsitnik]

@copilot please address the feedback where you have been tagged

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Unix.cs:373

• Same issue as above: using (pathBuilder) disposes a copy, which may not be the grown instance after ref mutations, and suppressing CS0728 hides a potential pooled-buffer leak. Prefer a using declaration on the local (or explicit try/finally) and avoid the extra nesting.

 string? path = Environment.ProcessPath;
ValueStringBuilder pathBuilder = new(stackalloc char[256]);
#pragma warning disable CS0728 // pathBuilder is intentionally mutated via ref; disposal still targets this local
using (pathBuilder)
{

[Copilot]

🟢 Approval recommended

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite

[adamsitnik]

/ba-g unrelated networking test failures

[Copilot]

• Path resolution
  • Move FindProgramInPath into the Unix implementation.
  • Build executable candidates with ValueStringBuilder; materialize strings only for successful matches.
  • Pass builder spans directly to executable checks via span-based UTF-8 interop.

Benchmark results from EgorBot/Benchmarks#551:

BenchmarkDotNet v0.16.0-preview.1, macOS Sequoia 15.4.1 (24E263) [Darwin 24.4.0]
Apple M4, 1 CPU, 10 logical and 10 physical cores
Memory: 16 GB Total, 4.36 GB Available
.NET SDK 11.0.100-rc.2.26426.116
[Host] : .NET 11.0.0 (11.0.0-rc.2.26426.116, 11.0.26.42716), Arm64 RyuJIT armv8.0-a

Method	Toolchain	Mean	Error	Ratio	Allocated	Alloc Ratio
Run	main	1,256.1 μs	2.72 μs	1.00	3.06 KB	2.43
Run	PR #132780	1,255.7 μs	2.76 μs	1.00	1.26 KB	1.00
Start	main	115.9 μs	2.56 μs	0.99	3.32 KB	2.19
Start	PR #132780	117.3 μs	3.29 μs	1.00	1.52 KB	1.00

BenchmarkDotNet v0.16.0-preview.1, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
AMD EPYC 9V45 2.60GHz, 1 CPU, 8 logical and 4 physical cores
Memory: 31.34 GB Total, 10.43 GB Available
.NET SDK 11.0.100-rc.2.26426.116
[Host] : .NET 11.0.0 (11.0.0-rc.2.26426.116, 11.0.26.42716), X64 RyuJIT x86-64-v4

Method	Toolchain	Mean	Error	Ratio	Allocated	Alloc Ratio
Run	main	633.4 μs	6.39 μs	1.00	2304 B	2.30
Run	PR #132780	631.3 μs	3.48 μs	1.00	1000 B	1.00
Start	main	156.9 μs	2.20 μs	0.93	2568 B	2.03
Start	PR #132780	169.4 μs	5.24 μs	1.00	1264 B	1.00

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[adamsitnik]

@copilot address my feedback

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[Copilot]

@copilot address my feedback

Addressed in a9eedc6: uses span-based Stat, centralizes path composition, and shares ResolvePath's builder with PATH lookup.

Note

AI/Copilot-generated response.

[adamsitnik]

@copilot it looks much better now. Please respond to my question

[adamsitnik]

@EgorBot -osx_arm64 -linux_x64

usingBenchmarkDotNet.Attributes;usingBenchmarkDotNet.Engines;usingBenchmarkDotNet.Running;usingMicrosoft.Win32.SafeHandles;usingSystem;usingSystem.Diagnostics;usingSystem.IO;usingSystem.Text;usingSystem.Threading.Tasks;BenchmarkSwitcher.FromAssembly(typeof(DefaultEnvVars).Assembly).Run(args);[MemoryDiagnoser]publicclassDefaultEnvVars{privatestaticreadonlySafeFileHandles_nullHandle=File.OpenNullHandle();privatestaticProcessStartInfos_startProcessStartInfo=newProcessStartInfo(){FileName="whoami",// exists on both Windows and Unix, and has very short outputStandardInputHandle=s_nullHandle,StandardOutputHandle=s_nullHandle,StandardErrorHandle=s_nullHandle,};privateProcess?_startedProcess;[Benchmark]publicvoidStart()=>_startedProcess=Process.Start(s_startProcessStartInfo)!;[IterationCleanup(Target=nameof(Start))]publicvoidCleanupStart(){if(_startedProcess!=null){_startedProcess.WaitForExit();_startedProcess.Dispose();_startedProcess=null;}}[Benchmark]publicintRun()=>Process.Run(s_startProcessStartInfo).ExitCode;[GlobalCleanup]publicvoidCleanup()=>s_nullHandle.Dispose();}

[adamsitnik]

@copilot please invoke the code-review skill and post your analysis/results as a comment on this PR

[Copilot]

@copilot please invoke the code-review skill and post your analysis/results as a comment on this PR

Holistic Review

Motivation: The code removes transient candidate-path strings from Unix executable lookup, a hot portion of Process.Start. The requested cross-architecture benchmark is the appropriate evidence for the claimed allocation reduction.

Approach: The change uses ValueStringBuilder with span-based UTF-8 marshalling, and materializes a string only after finding an executable. The ref-taking PATH helper correctly shares the builder with ResolvePath.

Summary: ⚠️ Needs Human Review. The code is sound after d7c184a, which resets the shared builder before PATH fallback; both independent reviewers found this same regression before the fix. The Process library Release build passes, but the requested benchmark results and focused test execution are not yet available for final performance and behavioral confirmation.

---

Detailed Findings

✅ Correctness — PATH fallback builder state

d7c184a clears the failed current-directory candidate before calling the ref-taking PATH helper and adds a debug assertion for its empty-builder contract. This fixes the prior behavior where the first PATH candidate was appended to the failed current-directory candidate.

✅ Allocation strategy and API surface

Candidate paths are composed in a stack-backed ValueStringBuilder; Access and Stat receive spans through the existing UTF-8 span marshaller. No public API surface was added. StringParser remains required by Linux procfs interop compiled into this project.

⚠️ Validation — benchmark and focused tests pending

The requested EgorBot benchmark has not produced results in the PR, so allocation improvement has not yet been measured on the requested Linux x64 and macOS arm64 configurations. The focused Process test project could not run locally because shared-framework/testhost prerequisites were unavailable; the Process library Release build did pass.

Note

AI/Copilot-generated review.

[adamsitnik]

@copilot please address the feedback where you have been tagged

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Unix.cs:373

• Same issue as above: using (pathBuilder) disposes a copy, which may not be the grown instance after ref mutations, and suppressing CS0728 hides a potential pooled-buffer leak. Prefer a using declaration on the local (or explicit try/finally) and avoid the extra nesting.

 string? path = Environment.ProcessPath;
ValueStringBuilder pathBuilder = new(stackalloc char[256]);
#pragma warning disable CS0728 // pathBuilder is intentionally mutated via ref; disposal still targets this local
using (pathBuilder)
{

[Copilot]

🟢 Approval recommended

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite

[adamsitnik]

/ba-g unrelated networking test failures

[Copilot]

• Path resolution
  • Move FindProgramInPath into the Unix implementation.
  • Build executable candidates with ValueStringBuilder; materialize strings only for successful matches.
  • Pass builder spans directly to executable checks via span-based UTF-8 interop.

Benchmark results from EgorBot/Benchmarks#551:

BenchmarkDotNet v0.16.0-preview.1, macOS Sequoia 15.4.1 (24E263) [Darwin 24.4.0]
Apple M4, 1 CPU, 10 logical and 10 physical cores
Memory: 16 GB Total, 4.36 GB Available
.NET SDK 11.0.100-rc.2.26426.116
[Host] : .NET 11.0.0 (11.0.0-rc.2.26426.116, 11.0.26.42716), Arm64 RyuJIT armv8.0-a

Method	Toolchain	Mean	Error	Ratio	Allocated	Alloc Ratio
Run	main	1,256.1 μs	2.72 μs	1.00	3.06 KB	2.43
Run	PR #132780	1,255.7 μs	2.76 μs	1.00	1.26 KB	1.00
Start	main	115.9 μs	2.56 μs	0.99	3.32 KB	2.19
Start	PR #132780	117.3 μs	3.29 μs	1.00	1.52 KB	1.00

BenchmarkDotNet v0.16.0-preview.1, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
AMD EPYC 9V45 2.60GHz, 1 CPU, 8 logical and 4 physical cores
Memory: 31.34 GB Total, 10.43 GB Available
.NET SDK 11.0.100-rc.2.26426.116
[Host] : .NET 11.0.0 (11.0.0-rc.2.26426.116, 11.0.26.42716), X64 RyuJIT x86-64-v4

Method	Toolchain	Mean	Error	Ratio	Allocated	Alloc Ratio
Run	main	633.4 μs	6.39 μs	1.00	2304 B	2.30
Run	PR #132780	631.3 μs	3.48 μs	1.00	1000 B	1.00
Start	main	156.9 μs	2.20 μs	0.93	2568 B	2.03
Start	PR #132780	169.4 μs	5.24 μs	1.00	1264 B	1.00

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[adamsitnik]

@copilot address my feedback

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[Copilot]

@copilot address my feedback

Addressed in a9eedc6: uses span-based Stat, centralizes path composition, and shares ResolvePath's builder with PATH lookup.

Note

AI/Copilot-generated response.

[adamsitnik]

@copilot it looks much better now. Please respond to my question

[adamsitnik]

@EgorBot -osx_arm64 -linux_x64

usingBenchmarkDotNet.Attributes;usingBenchmarkDotNet.Engines;usingBenchmarkDotNet.Running;usingMicrosoft.Win32.SafeHandles;usingSystem;usingSystem.Diagnostics;usingSystem.IO;usingSystem.Text;usingSystem.Threading.Tasks;BenchmarkSwitcher.FromAssembly(typeof(DefaultEnvVars).Assembly).Run(args);[MemoryDiagnoser]publicclassDefaultEnvVars{privatestaticreadonlySafeFileHandles_nullHandle=File.OpenNullHandle();privatestaticProcessStartInfos_startProcessStartInfo=newProcessStartInfo(){FileName="whoami",// exists on both Windows and Unix, and has very short outputStandardInputHandle=s_nullHandle,StandardOutputHandle=s_nullHandle,StandardErrorHandle=s_nullHandle,};privateProcess?_startedProcess;[Benchmark]publicvoidStart()=>_startedProcess=Process.Start(s_startProcessStartInfo)!;[IterationCleanup(Target=nameof(Start))]publicvoidCleanupStart(){if(_startedProcess!=null){_startedProcess.WaitForExit();_startedProcess.Dispose();_startedProcess=null;}}[Benchmark]publicintRun()=>Process.Run(s_startProcessStartInfo).ExitCode;[GlobalCleanup]publicvoidCleanup()=>s_nullHandle.Dispose();}

[adamsitnik]

@copilot please invoke the code-review skill and post your analysis/results as a comment on this PR

[Copilot]

@copilot please invoke the code-review skill and post your analysis/results as a comment on this PR

Holistic Review

Motivation: The code removes transient candidate-path strings from Unix executable lookup, a hot portion of Process.Start. The requested cross-architecture benchmark is the appropriate evidence for the claimed allocation reduction.

Approach: The change uses ValueStringBuilder with span-based UTF-8 marshalling, and materializes a string only after finding an executable. The ref-taking PATH helper correctly shares the builder with ResolvePath.

Summary: ⚠️ Needs Human Review. The code is sound after d7c184a, which resets the shared builder before PATH fallback; both independent reviewers found this same regression before the fix. The Process library Release build passes, but the requested benchmark results and focused test execution are not yet available for final performance and behavioral confirmation.

---

Detailed Findings

✅ Correctness — PATH fallback builder state

d7c184a clears the failed current-directory candidate before calling the ref-taking PATH helper and adds a debug assertion for its empty-builder contract. This fixes the prior behavior where the first PATH candidate was appended to the failed current-directory candidate.

✅ Allocation strategy and API surface

Candidate paths are composed in a stack-backed ValueStringBuilder; Access and Stat receive spans through the existing UTF-8 span marshaller. No public API surface was added. StringParser remains required by Linux procfs interop compiled into this project.

⚠️ Validation — benchmark and focused tests pending

The requested EgorBot benchmark has not produced results in the PR, so allocation improvement has not yet been measured on the requested Linux x64 and macOS arm64 configurations. The focused Process test project could not run locally because shared-framework/testhost prerequisites were unavailable; the Process library Release build did pass.

Note

AI/Copilot-generated review.

[adamsitnik]

@copilot please address the feedback where you have been tagged

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Unix.cs:373

• Same issue as above: using (pathBuilder) disposes a copy, which may not be the grown instance after ref mutations, and suppressing CS0728 hides a potential pooled-buffer leak. Prefer a using declaration on the local (or explicit try/finally) and avoid the extra nesting.

 string? path = Environment.ProcessPath;
ValueStringBuilder pathBuilder = new(stackalloc char[256]);
#pragma warning disable CS0728 // pathBuilder is intentionally mutated via ref; disposal still targets this local
using (pathBuilder)
{

[Copilot]

🟢 Approval recommended

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite

[adamsitnik]

/ba-g unrelated networking test failures

[Copilot]

• Path resolution
  • Move FindProgramInPath into the Unix implementation.
  • Build executable candidates with ValueStringBuilder; materialize strings only for successful matches.
  • Pass builder spans directly to executable checks via span-based UTF-8 interop.

Benchmark results from EgorBot/Benchmarks#551:

BenchmarkDotNet v0.16.0-preview.1, macOS Sequoia 15.4.1 (24E263) [Darwin 24.4.0]
Apple M4, 1 CPU, 10 logical and 10 physical cores
Memory: 16 GB Total, 4.36 GB Available
.NET SDK 11.0.100-rc.2.26426.116
[Host] : .NET 11.0.0 (11.0.0-rc.2.26426.116, 11.0.26.42716), Arm64 RyuJIT armv8.0-a

Method	Toolchain	Mean	Error	Ratio	Allocated	Alloc Ratio
Run	main	1,256.1 μs	2.72 μs	1.00	3.06 KB	2.43
Run	PR #132780	1,255.7 μs	2.76 μs	1.00	1.26 KB	1.00
Start	main	115.9 μs	2.56 μs	0.99	3.32 KB	2.19
Start	PR #132780	117.3 μs	3.29 μs	1.00	1.52 KB	1.00

BenchmarkDotNet v0.16.0-preview.1, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
AMD EPYC 9V45 2.60GHz, 1 CPU, 8 logical and 4 physical cores
Memory: 31.34 GB Total, 10.43 GB Available
.NET SDK 11.0.100-rc.2.26426.116
[Host] : .NET 11.0.0 (11.0.0-rc.2.26426.116, 11.0.26.42716), X64 RyuJIT x86-64-v4

Method	Toolchain	Mean	Error	Ratio	Allocated	Alloc Ratio
Run	main	633.4 μs	6.39 μs	1.00	2304 B	2.30
Run	PR #132780	631.3 μs	3.48 μs	1.00	1000 B	1.00
Start	main	156.9 μs	2.20 μs	0.93	2568 B	2.03
Start	PR #132780	169.4 μs	5.24 μs	1.00	1264 B	1.00

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[adamsitnik]

@copilot address my feedback

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[Copilot]

@copilot address my feedback

Addressed in a9eedc6: uses span-based Stat, centralizes path composition, and shares ResolvePath's builder with PATH lookup.

Note

AI/Copilot-generated response.

[adamsitnik]

@copilot it looks much better now. Please respond to my question

[adamsitnik]

@EgorBot -osx_arm64 -linux_x64

usingBenchmarkDotNet.Attributes;usingBenchmarkDotNet.Engines;usingBenchmarkDotNet.Running;usingMicrosoft.Win32.SafeHandles;usingSystem;usingSystem.Diagnostics;usingSystem.IO;usingSystem.Text;usingSystem.Threading.Tasks;BenchmarkSwitcher.FromAssembly(typeof(DefaultEnvVars).Assembly).Run(args);[MemoryDiagnoser]publicclassDefaultEnvVars{privatestaticreadonlySafeFileHandles_nullHandle=File.OpenNullHandle();privatestaticProcessStartInfos_startProcessStartInfo=newProcessStartInfo(){FileName="whoami",// exists on both Windows and Unix, and has very short outputStandardInputHandle=s_nullHandle,StandardOutputHandle=s_nullHandle,StandardErrorHandle=s_nullHandle,};privateProcess?_startedProcess;[Benchmark]publicvoidStart()=>_startedProcess=Process.Start(s_startProcessStartInfo)!;[IterationCleanup(Target=nameof(Start))]publicvoidCleanupStart(){if(_startedProcess!=null){_startedProcess.WaitForExit();_startedProcess.Dispose();_startedProcess=null;}}[Benchmark]publicintRun()=>Process.Run(s_startProcessStartInfo).ExitCode;[GlobalCleanup]publicvoidCleanup()=>s_nullHandle.Dispose();}

[adamsitnik]

@copilot please invoke the code-review skill and post your analysis/results as a comment on this PR

[Copilot]

@copilot please invoke the code-review skill and post your analysis/results as a comment on this PR

Holistic Review

Motivation: The code removes transient candidate-path strings from Unix executable lookup, a hot portion of Process.Start. The requested cross-architecture benchmark is the appropriate evidence for the claimed allocation reduction.

Approach: The change uses ValueStringBuilder with span-based UTF-8 marshalling, and materializes a string only after finding an executable. The ref-taking PATH helper correctly shares the builder with ResolvePath.

Summary: ⚠️ Needs Human Review. The code is sound after d7c184a, which resets the shared builder before PATH fallback; both independent reviewers found this same regression before the fix. The Process library Release build passes, but the requested benchmark results and focused test execution are not yet available for final performance and behavioral confirmation.

---

Detailed Findings

✅ Correctness — PATH fallback builder state

d7c184a clears the failed current-directory candidate before calling the ref-taking PATH helper and adds a debug assertion for its empty-builder contract. This fixes the prior behavior where the first PATH candidate was appended to the failed current-directory candidate.

✅ Allocation strategy and API surface

Candidate paths are composed in a stack-backed ValueStringBuilder; Access and Stat receive spans through the existing UTF-8 span marshaller. No public API surface was added. StringParser remains required by Linux procfs interop compiled into this project.

⚠️ Validation — benchmark and focused tests pending

The requested EgorBot benchmark has not produced results in the PR, so allocation improvement has not yet been measured on the requested Linux x64 and macOS arm64 configurations. The focused Process test project could not run locally because shared-framework/testhost prerequisites were unavailable; the Process library Release build did pass.

Note

AI/Copilot-generated review.

[adamsitnik]

@copilot please address the feedback where you have been tagged

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Unix.cs:373

• Same issue as above: using (pathBuilder) disposes a copy, which may not be the grown instance after ref mutations, and suppressing CS0728 hides a potential pooled-buffer leak. Prefer a using declaration on the local (or explicit try/finally) and avoid the extra nesting.

 string? path = Environment.ProcessPath;
ValueStringBuilder pathBuilder = new(stackalloc char[256]);
#pragma warning disable CS0728 // pathBuilder is intentionally mutated via ref; disposal still targets this local
using (pathBuilder)
{

[Copilot]

🟢 Approval recommended

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite

[adamsitnik]

/ba-g unrelated networking test failures

[Copilot]

• Path resolution
  • Move FindProgramInPath into the Unix implementation.
  • Build executable candidates with ValueStringBuilder; materialize strings only for successful matches.
  • Pass builder spans directly to executable checks via span-based UTF-8 interop.

Benchmark results from EgorBot/Benchmarks#551:

BenchmarkDotNet v0.16.0-preview.1, macOS Sequoia 15.4.1 (24E263) [Darwin 24.4.0]
Apple M4, 1 CPU, 10 logical and 10 physical cores
Memory: 16 GB Total, 4.36 GB Available
.NET SDK 11.0.100-rc.2.26426.116
[Host] : .NET 11.0.0 (11.0.0-rc.2.26426.116, 11.0.26.42716), Arm64 RyuJIT armv8.0-a

Method	Toolchain	Mean	Error	Ratio	Allocated	Alloc Ratio
Run	main	1,256.1 μs	2.72 μs	1.00	3.06 KB	2.43
Run	PR #132780	1,255.7 μs	2.76 μs	1.00	1.26 KB	1.00
Start	main	115.9 μs	2.56 μs	0.99	3.32 KB	2.19
Start	PR #132780	117.3 μs	3.29 μs	1.00	1.52 KB	1.00

BenchmarkDotNet v0.16.0-preview.1, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
AMD EPYC 9V45 2.60GHz, 1 CPU, 8 logical and 4 physical cores
Memory: 31.34 GB Total, 10.43 GB Available
.NET SDK 11.0.100-rc.2.26426.116
[Host] : .NET 11.0.0 (11.0.0-rc.2.26426.116, 11.0.26.42716), X64 RyuJIT x86-64-v4

Method	Toolchain	Mean	Error	Ratio	Allocated	Alloc Ratio
Run	main	633.4 μs	6.39 μs	1.00	2304 B	2.30
Run	PR #132780	631.3 μs	3.48 μs	1.00	1000 B	1.00
Start	main	156.9 μs	2.20 μs	0.93	2568 B	2.03
Start	PR #132780	169.4 μs	5.24 μs	1.00	1264 B	1.00

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[adamsitnik]

@copilot address my feedback

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[Copilot]

@copilot address my feedback

Addressed in a9eedc6: uses span-based Stat, centralizes path composition, and shares ResolvePath's builder with PATH lookup.

Note

AI/Copilot-generated response.

[adamsitnik]

@copilot it looks much better now. Please respond to my question

[adamsitnik]

@EgorBot -osx_arm64 -linux_x64

usingBenchmarkDotNet.Attributes;usingBenchmarkDotNet.Engines;usingBenchmarkDotNet.Running;usingMicrosoft.Win32.SafeHandles;usingSystem;usingSystem.Diagnostics;usingSystem.IO;usingSystem.Text;usingSystem.Threading.Tasks;BenchmarkSwitcher.FromAssembly(typeof(DefaultEnvVars).Assembly).Run(args);[MemoryDiagnoser]publicclassDefaultEnvVars{privatestaticreadonlySafeFileHandles_nullHandle=File.OpenNullHandle();privatestaticProcessStartInfos_startProcessStartInfo=newProcessStartInfo(){FileName="whoami",// exists on both Windows and Unix, and has very short outputStandardInputHandle=s_nullHandle,StandardOutputHandle=s_nullHandle,StandardErrorHandle=s_nullHandle,};privateProcess?_startedProcess;[Benchmark]publicvoidStart()=>_startedProcess=Process.Start(s_startProcessStartInfo)!;[IterationCleanup(Target=nameof(Start))]publicvoidCleanupStart(){if(_startedProcess!=null){_startedProcess.WaitForExit();_startedProcess.Dispose();_startedProcess=null;}}[Benchmark]publicintRun()=>Process.Run(s_startProcessStartInfo).ExitCode;[GlobalCleanup]publicvoidCleanup()=>s_nullHandle.Dispose();}

[adamsitnik]

@copilot please invoke the code-review skill and post your analysis/results as a comment on this PR

[Copilot]

@copilot please invoke the code-review skill and post your analysis/results as a comment on this PR

Holistic Review

Motivation: The code removes transient candidate-path strings from Unix executable lookup, a hot portion of Process.Start. The requested cross-architecture benchmark is the appropriate evidence for the claimed allocation reduction.

Approach: The change uses ValueStringBuilder with span-based UTF-8 marshalling, and materializes a string only after finding an executable. The ref-taking PATH helper correctly shares the builder with ResolvePath.

Summary: ⚠️ Needs Human Review. The code is sound after d7c184a, which resets the shared builder before PATH fallback; both independent reviewers found this same regression before the fix. The Process library Release build passes, but the requested benchmark results and focused test execution are not yet available for final performance and behavioral confirmation.

---

Detailed Findings

✅ Correctness — PATH fallback builder state

d7c184a clears the failed current-directory candidate before calling the ref-taking PATH helper and adds a debug assertion for its empty-builder contract. This fixes the prior behavior where the first PATH candidate was appended to the failed current-directory candidate.

✅ Allocation strategy and API surface

Candidate paths are composed in a stack-backed ValueStringBuilder; Access and Stat receive spans through the existing UTF-8 span marshaller. No public API surface was added. StringParser remains required by Linux procfs interop compiled into this project.

⚠️ Validation — benchmark and focused tests pending

The requested EgorBot benchmark has not produced results in the PR, so allocation improvement has not yet been measured on the requested Linux x64 and macOS arm64 configurations. The focused Process test project could not run locally because shared-framework/testhost prerequisites were unavailable; the Process library Release build did pass.

Note

AI/Copilot-generated review.

[adamsitnik]

@copilot please address the feedback where you have been tagged

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Unix.cs:373

• Same issue as above: using (pathBuilder) disposes a copy, which may not be the grown instance after ref mutations, and suppressing CS0728 hides a potential pooled-buffer leak. Prefer a using declaration on the local (or explicit try/finally) and avoid the extra nesting.

 string? path = Environment.ProcessPath;
ValueStringBuilder pathBuilder = new(stackalloc char[256]);
#pragma warning disable CS0728 // pathBuilder is intentionally mutated via ref; disposal still targets this local
using (pathBuilder)
{

[Copilot]

🟢 Approval recommended

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite

[adamsitnik]

/ba-g unrelated networking test failures

[Copilot]

• Path resolution
  • Move FindProgramInPath into the Unix implementation.
  • Build executable candidates with ValueStringBuilder; materialize strings only for successful matches.
  • Pass builder spans directly to executable checks via span-based UTF-8 interop.

Benchmark results from EgorBot/Benchmarks#551:

BenchmarkDotNet v0.16.0-preview.1, macOS Sequoia 15.4.1 (24E263) [Darwin 24.4.0]
Apple M4, 1 CPU, 10 logical and 10 physical cores
Memory: 16 GB Total, 4.36 GB Available
.NET SDK 11.0.100-rc.2.26426.116
[Host] : .NET 11.0.0 (11.0.0-rc.2.26426.116, 11.0.26.42716), Arm64 RyuJIT armv8.0-a

Method	Toolchain	Mean	Error	Ratio	Allocated	Alloc Ratio
Run	main	1,256.1 μs	2.72 μs	1.00	3.06 KB	2.43
Run	PR #132780	1,255.7 μs	2.76 μs	1.00	1.26 KB	1.00
Start	main	115.9 μs	2.56 μs	0.99	3.32 KB	2.19
Start	PR #132780	117.3 μs	3.29 μs	1.00	1.52 KB	1.00

BenchmarkDotNet v0.16.0-preview.1, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
AMD EPYC 9V45 2.60GHz, 1 CPU, 8 logical and 4 physical cores
Memory: 31.34 GB Total, 10.43 GB Available
.NET SDK 11.0.100-rc.2.26426.116
[Host] : .NET 11.0.0 (11.0.0-rc.2.26426.116, 11.0.26.42716), X64 RyuJIT x86-64-v4

Method	Toolchain	Mean	Error	Ratio	Allocated	Alloc Ratio
Run	main	633.4 μs	6.39 μs	1.00	2304 B	2.30
Run	PR #132780	631.3 μs	3.48 μs	1.00	1000 B	1.00
Start	main	156.9 μs	2.20 μs	0.93	2568 B	2.03
Start	PR #132780	169.4 μs	5.24 μs	1.00	1264 B	1.00

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[adamsitnik]

@copilot address my feedback

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[Copilot]

@copilot address my feedback

Addressed in a9eedc6: uses span-based Stat, centralizes path composition, and shares ResolvePath's builder with PATH lookup.

Note

AI/Copilot-generated response.

[adamsitnik]

@copilot it looks much better now. Please respond to my question

[adamsitnik]

@EgorBot -osx_arm64 -linux_x64

usingBenchmarkDotNet.Attributes;usingBenchmarkDotNet.Engines;usingBenchmarkDotNet.Running;usingMicrosoft.Win32.SafeHandles;usingSystem;usingSystem.Diagnostics;usingSystem.IO;usingSystem.Text;usingSystem.Threading.Tasks;BenchmarkSwitcher.FromAssembly(typeof(DefaultEnvVars).Assembly).Run(args);[MemoryDiagnoser]publicclassDefaultEnvVars{privatestaticreadonlySafeFileHandles_nullHandle=File.OpenNullHandle();privatestaticProcessStartInfos_startProcessStartInfo=newProcessStartInfo(){FileName="whoami",// exists on both Windows and Unix, and has very short outputStandardInputHandle=s_nullHandle,StandardOutputHandle=s_nullHandle,StandardErrorHandle=s_nullHandle,};privateProcess?_startedProcess;[Benchmark]publicvoidStart()=>_startedProcess=Process.Start(s_startProcessStartInfo)!;[IterationCleanup(Target=nameof(Start))]publicvoidCleanupStart(){if(_startedProcess!=null){_startedProcess.WaitForExit();_startedProcess.Dispose();_startedProcess=null;}}[Benchmark]publicintRun()=>Process.Run(s_startProcessStartInfo).ExitCode;[GlobalCleanup]publicvoidCleanup()=>s_nullHandle.Dispose();}

[adamsitnik]

@copilot please invoke the code-review skill and post your analysis/results as a comment on this PR

[Copilot]

@copilot please invoke the code-review skill and post your analysis/results as a comment on this PR

Holistic Review

Motivation: The code removes transient candidate-path strings from Unix executable lookup, a hot portion of Process.Start. The requested cross-architecture benchmark is the appropriate evidence for the claimed allocation reduction.

Approach: The change uses ValueStringBuilder with span-based UTF-8 marshalling, and materializes a string only after finding an executable. The ref-taking PATH helper correctly shares the builder with ResolvePath.

Summary: ⚠️ Needs Human Review. The code is sound after d7c184a, which resets the shared builder before PATH fallback; both independent reviewers found this same regression before the fix. The Process library Release build passes, but the requested benchmark results and focused test execution are not yet available for final performance and behavioral confirmation.

---

Detailed Findings

✅ Correctness — PATH fallback builder state

d7c184a clears the failed current-directory candidate before calling the ref-taking PATH helper and adds a debug assertion for its empty-builder contract. This fixes the prior behavior where the first PATH candidate was appended to the failed current-directory candidate.

✅ Allocation strategy and API surface

Candidate paths are composed in a stack-backed ValueStringBuilder; Access and Stat receive spans through the existing UTF-8 span marshaller. No public API surface was added. StringParser remains required by Linux procfs interop compiled into this project.

⚠️ Validation — benchmark and focused tests pending

The requested EgorBot benchmark has not produced results in the PR, so allocation improvement has not yet been measured on the requested Linux x64 and macOS arm64 configurations. The focused Process test project could not run locally because shared-framework/testhost prerequisites were unavailable; the Process library Release build did pass.

Note

AI/Copilot-generated review.

[adamsitnik]

@copilot please address the feedback where you have been tagged

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Unix.cs:373

• Same issue as above: using (pathBuilder) disposes a copy, which may not be the grown instance after ref mutations, and suppressing CS0728 hides a potential pooled-buffer leak. Prefer a using declaration on the local (or explicit try/finally) and avoid the extra nesting.

 string? path = Environment.ProcessPath;
ValueStringBuilder pathBuilder = new(stackalloc char[256]);
#pragma warning disable CS0728 // pathBuilder is intentionally mutated via ref; disposal still targets this local
using (pathBuilder)
{

[Copilot]

🟢 Approval recommended

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite

[adamsitnik]

/ba-g unrelated networking test failures

[Copilot]

• Path resolution
  • Move FindProgramInPath into the Unix implementation.
  • Build executable candidates with ValueStringBuilder; materialize strings only for successful matches.
  • Pass builder spans directly to executable checks via span-based UTF-8 interop.

Benchmark results from EgorBot/Benchmarks#551:

BenchmarkDotNet v0.16.0-preview.1, macOS Sequoia 15.4.1 (24E263) [Darwin 24.4.0]
Apple M4, 1 CPU, 10 logical and 10 physical cores
Memory: 16 GB Total, 4.36 GB Available
.NET SDK 11.0.100-rc.2.26426.116
[Host] : .NET 11.0.0 (11.0.0-rc.2.26426.116, 11.0.26.42716), Arm64 RyuJIT armv8.0-a

Method	Toolchain	Mean	Error	Ratio	Allocated	Alloc Ratio
Run	main	1,256.1 μs	2.72 μs	1.00	3.06 KB	2.43
Run	PR #132780	1,255.7 μs	2.76 μs	1.00	1.26 KB	1.00
Start	main	115.9 μs	2.56 μs	0.99	3.32 KB	2.19
Start	PR #132780	117.3 μs	3.29 μs	1.00	1.52 KB	1.00

BenchmarkDotNet v0.16.0-preview.1, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
AMD EPYC 9V45 2.60GHz, 1 CPU, 8 logical and 4 physical cores
Memory: 31.34 GB Total, 10.43 GB Available
.NET SDK 11.0.100-rc.2.26426.116
[Host] : .NET 11.0.0 (11.0.0-rc.2.26426.116, 11.0.26.42716), X64 RyuJIT x86-64-v4

Method	Toolchain	Mean	Error	Ratio	Allocated	Alloc Ratio
Run	main	633.4 μs	6.39 μs	1.00	2304 B	2.30
Run	PR #132780	631.3 μs	3.48 μs	1.00	1000 B	1.00
Start	main	156.9 μs	2.20 μs	0.93	2568 B	2.03
Start	PR #132780	169.4 μs	5.24 μs	1.00	1264 B	1.00

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[adamsitnik]

@copilot address my feedback

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[Copilot]

@copilot address my feedback

Addressed in a9eedc6: uses span-based Stat, centralizes path composition, and shares ResolvePath's builder with PATH lookup.

Note

AI/Copilot-generated response.

[adamsitnik]

@copilot it looks much better now. Please respond to my question

[adamsitnik]

@EgorBot -osx_arm64 -linux_x64

usingBenchmarkDotNet.Attributes;usingBenchmarkDotNet.Engines;usingBenchmarkDotNet.Running;usingMicrosoft.Win32.SafeHandles;usingSystem;usingSystem.Diagnostics;usingSystem.IO;usingSystem.Text;usingSystem.Threading.Tasks;BenchmarkSwitcher.FromAssembly(typeof(DefaultEnvVars).Assembly).Run(args);[MemoryDiagnoser]publicclassDefaultEnvVars{privatestaticreadonlySafeFileHandles_nullHandle=File.OpenNullHandle();privatestaticProcessStartInfos_startProcessStartInfo=newProcessStartInfo(){FileName="whoami",// exists on both Windows and Unix, and has very short outputStandardInputHandle=s_nullHandle,StandardOutputHandle=s_nullHandle,StandardErrorHandle=s_nullHandle,};privateProcess?_startedProcess;[Benchmark]publicvoidStart()=>_startedProcess=Process.Start(s_startProcessStartInfo)!;[IterationCleanup(Target=nameof(Start))]publicvoidCleanupStart(){if(_startedProcess!=null){_startedProcess.WaitForExit();_startedProcess.Dispose();_startedProcess=null;}}[Benchmark]publicintRun()=>Process.Run(s_startProcessStartInfo).ExitCode;[GlobalCleanup]publicvoidCleanup()=>s_nullHandle.Dispose();}

[adamsitnik]

@copilot please invoke the code-review skill and post your analysis/results as a comment on this PR

[Copilot]

@copilot please invoke the code-review skill and post your analysis/results as a comment on this PR

Holistic Review

Motivation: The code removes transient candidate-path strings from Unix executable lookup, a hot portion of Process.Start. The requested cross-architecture benchmark is the appropriate evidence for the claimed allocation reduction.

Approach: The change uses ValueStringBuilder with span-based UTF-8 marshalling, and materializes a string only after finding an executable. The ref-taking PATH helper correctly shares the builder with ResolvePath.

Summary: ⚠️ Needs Human Review. The code is sound after d7c184a, which resets the shared builder before PATH fallback; both independent reviewers found this same regression before the fix. The Process library Release build passes, but the requested benchmark results and focused test execution are not yet available for final performance and behavioral confirmation.

---

Detailed Findings

✅ Correctness — PATH fallback builder state

d7c184a clears the failed current-directory candidate before calling the ref-taking PATH helper and adds a debug assertion for its empty-builder contract. This fixes the prior behavior where the first PATH candidate was appended to the failed current-directory candidate.

✅ Allocation strategy and API surface

Candidate paths are composed in a stack-backed ValueStringBuilder; Access and Stat receive spans through the existing UTF-8 span marshaller. No public API surface was added. StringParser remains required by Linux procfs interop compiled into this project.

⚠️ Validation — benchmark and focused tests pending

The requested EgorBot benchmark has not produced results in the PR, so allocation improvement has not yet been measured on the requested Linux x64 and macOS arm64 configurations. The focused Process test project could not run locally because shared-framework/testhost prerequisites were unavailable; the Process library Release build did pass.

Note

AI/Copilot-generated review.

[adamsitnik]

@copilot please address the feedback where you have been tagged

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Unix.cs:373

• Same issue as above: using (pathBuilder) disposes a copy, which may not be the grown instance after ref mutations, and suppressing CS0728 hides a potential pooled-buffer leak. Prefer a using declaration on the local (or explicit try/finally) and avoid the extra nesting.

 string? path = Environment.ProcessPath;
ValueStringBuilder pathBuilder = new(stackalloc char[256]);
#pragma warning disable CS0728 // pathBuilder is intentionally mutated via ref; disposal still targets this local
using (pathBuilder)
{

[Copilot]

🟢 Approval recommended

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite

[adamsitnik]

/ba-g unrelated networking test failures