-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Improve throughput of Environment.GetEnvironmentVariables() #45057
Conversation
src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Windows.cs
Outdated
Show resolved
Hide resolved
0a25ea4
to
ed592e4
Compare
src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Windows.cs
Outdated
Show resolved
Hide resolved
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.
Nice!
src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetEnvironmentStrings.cs
Show resolved
Hide resolved
ed592e4
to
b87f3a3
Compare
src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FreeEnvironmentStrings.cs
Outdated
Show resolved
Hide resolved
Use IndexOf to search for positions rather than open-coded loops, taking advantage of vectorization to improve throughput.
b87f3a3
to
8fe2a9b
Compare
@stephentoub do you have any plans on improving the Unix implementation as well? This could improve the developer loop experience on macOS |
@adamsitnik, |
@am11 it might be, but according to the data shared in #41871, getting a single env var on macOS is still 4 times slower compared to Windows (using same hardware). And the time is not spent in the method that @stephentoub has optimized in this particular PR (see #866 (comment) for details) |
As @am11 said, this is the implementation of GetEnvironmentVariables for both Windows and Unix. GetEnvironmentVariable (singular) doesn't use this, on any platform. |
Are there any plans to improve the performance of Unix implementation of the GetEnvironmentVariable (singular) method? |
The costs you highlight for GetEnvironmentVariable are because the runtime reads and stores the UTF8 env vars on first use, and then calls to GetEnvironmentVariable need to transcode the UTF16 key being searched for, which is done linearly. To make GetEnvironmentVariable faster in microbenchmarks, the runtime could instead pre-transcode them all and potentially store them in a hashtable. The problem, though, is trying to optimize for microbenchmarks on the throughput of GetEnvironmentVariable will likely lead you to do the "wrong" thing. No one should be calling GetEnvironmentVariable on a hot path; instead, while there are exceptions to this, it's generally used once for a given key, often during startup / first use of some code that checks it for configuration info and then never looks at it again; in my experience, there are also generally many more environment variables in the environment than the app actually cares about. We could spend way more time transcoding all env vars, find we made a benchmark.net test of GetEnvironmentVariable much faster, but actually slowed down the overall startup of an app with no measurable reward to show for it. If you'd like to prototype that and can demonstrate it does help with the overall startup of various app types (e.g. because it turns out we end up looking for lots of environment variables, maybe multiple times, or because we have to transcode them all anyway for an inevitable call to GetEnvironmentVariables, plural), then we could certainly consider that direction. It would make multiple calls to GetEnvironmentVariables faster as well. (It's also possible there are some simpler, smaller optimizations possible. For example, if the vast majority of keys are ASCII, maybe there could be a fast-path that skips the transcoding initially and tries to do the comparison between the byte and char unless a non-ASCII value is hit.) |
Perf lab results shows improvements - DrewScoggins/performance-2#3515 |
Use IndexOf{Any} to search for positions rather than open-coded loops, taking advantage of vectorization to improve throughput.
With the environment variables in my command prompt:
Contributes to #44598