-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Remove some overhead from Process.Start #44691
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Tagging subscribers to this area: @eiriktsarpalis, @jeffhandley DetailsIssue Details
|
src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DuplicateHandle_SafeProcessHandle.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj
Outdated
Show resolved
Hide resolved
...aries/System.Diagnostics.PerformanceCounter/src/System.Diagnostics.PerformanceCounter.csproj
Outdated
Show resolved
Hide resolved
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs
Outdated
Show resolved
Hide resolved
2713e01 to
7d480dd
Compare
src/libraries/System.Security.Principal.Windows/src/System.Security.Principal.Windows.csproj
Outdated
Show resolved
Hide resolved
7d480dd to
ffb28f7
Compare
ffb28f7 to
0ca7ba1
Compare
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/PasteArguments.Unix.cs
Outdated
Show resolved
Hide resolved
eiriktsarpalis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
|
/azp run runtime-libraries outerloop |
|
No pipelines are associated with this pull request. |
src/libraries/System.Private.CoreLib/src/System/PasteArguments.Unix.cs
Outdated
Show resolved
Hide resolved
|
Build break |
- 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
ba1ecbd to
8b2bab4
Compare
What happens when you search within *.csproj and forget about *.projitems ;-) |


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