Remove some overhead from Process.Start by stephentoub · Pull Request #44691 · dotnet/runtime

stephentoub · 2020-11-14T19:29:06Z

Avoid StringBuilder marshaling
Let CreateProcess{WithLogonW} determine the current working directory
Avoid creating SafeHandles for GetCurrentProcess()
Avoid forcing ProcessStartInfo.ArgumentList into existence just to check if it contains anything
Prefer using ProcessStartInfo.Arguments in Start(string, IEnumerable) if there's only one string in the enumerable
Use ValueStringBuilder instead of StringBuilder to build up arguments, so as to use stack space / pooled char[]s and avoid actually needing to produce strings.
Avoid unnecessary SafeHandle for PROCESS_INFORMATION.hThread

Test that launches a thousand processes, no redirection, one argument (execution time is dominated by the actual native call and remote process, so this has a negligible impact on serial execution time):

Allocations before

Allocations after

Contributes to #44598

ghost · 2020-11-14T19:29:11Z

Tagging subscribers to this area: @eiriktsarpalis, @jeffhandley
See info in area-owners.md if you want to be subscribed.

Details

Issue Details

Description:	Avoid StringBuilder marshaling Let CreateProcess{WithLogonW} determine the current working directory Avoid creating SafeHandles for GetCurrentProcess() Avoid forcing ProcessStartInfo.ArgumentList into existence just to check if it contains anything Prefer using ProcessStartInfo.Arguments in Start(string, IEnumerable) if there's only one string in the enumerable Use ValueStringBuilder instead of StringBuilder to build up arguments, so as to use stack space / pooled char[]s and avoid actually needing to produce strings. Avoid unnecessary SafeHandle for PROCESS_INFORMATION.hThread Test that launches a thousand processes, no redirection, one argument (execution time is dominated by the actual native call and remote process, so this has a negligible impact on serial execution time): Allocations before Allocations after
Author:	stephentoub
Assignees:	-
Labels:	`area-System.Diagnostics.Process`
Milestone:	-

src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DuplicateHandle_SafeProcessHandle.cs

src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj

...aries/System.Diagnostics.PerformanceCounter/src/System.Diagnostics.PerformanceCounter.csproj

src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs

src/libraries/System.Security.Principal.Windows/src/System.Security.Principal.Windows.csproj

src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs

src/libraries/System.Private.CoreLib/src/System/PasteArguments.Unix.cs

eiriktsarpalis

LGTM.

eiriktsarpalis · 2020-11-16T16:20:37Z

/azp run runtime-libraries outerloop

azure-pipelines · 2020-11-16T16:20:42Z

No pipelines are associated with this pull request.

src/libraries/System.Private.CoreLib/src/System/PasteArguments.Unix.cs

src/libraries/System.IO.Pipes/src/System.IO.Pipes.csproj

jkotas · 2020-11-17T07:28:15Z

Build break

[F:\workspace\_work\1\s\src\coreclr\src\System.Private.CoreLib\System.Private.CoreLib.csproj]
Source file 'F:\workspace\_work\1\s\src\libraries\Common\src\Interop\Windows\Kernel32\Interop.GetCurrentProcess_IntPtr.cs' could not be found.

- Avoid StringBuilder marshaling - Let CreateProcess{WithLogonW} determine the current working directory - Avoid creating SafeHandles for GetCurrentProcess() - Avoid forcing ProcessStartInfo.ArgumentList into existence just to check if it contains anything - Prefer using ProcessStartInfo.Arguments in Start(string, IEnumerable<string>) if there's only one string in the enumerable - Use ValueStringBuilder instead of StringBuilder to build up arguments, so as to use stack space / pooled char[]s and avoid actually needing to produce strings. - Avoid unnecessary SafeHandle for PROCESS_INFORMATION.hThread

stephentoub · 2020-11-17T14:44:02Z

Build break

What happens when you search within *.csproj and forget about *.projitems ;-)

Dotnet-GitSync-Bot added the area-System.Diagnostics.Process label Nov 14, 2020