Skip to content
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

Investigate which other properties of System.Environment can be cached. #57442

Open
teo-tsirpanis opened this issue Aug 15, 2021 · 6 comments
Open
Labels
area-System.Runtime needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration
Milestone

Comments

@teo-tsirpanis
Copy link
Contributor

I noticed that the following System.Environment properties (on Windows at least) are not cached and instead perform syscalls and allocations to return a value that does not change over the course of a program's lifetime:

  • CommandLine - partially cached, since it does not always call into native code, but it still every time creates a string and calls the public GetCommandLineArgs which needlessly clones an internal array (that clone could also be eliminated when this property gets initialized).
  • CurrentDirectory - this cannot be entirely cached since it might change by directly calling the Windows APIs but we could check whether the current directory has changed since the last time we got this property and avoid a string allocation if it hasn't, returning a cached string object.
  • MachineName - calls a Windows API function and allocates a new string every time. Documentation says that "This name is established at system startup, when the system reads it from the registry."
  • SystemDirectory - calls a Windows API function and allocates a new string every time. Doubt it can change without reinstalling Windows.
  • SystemPageSize - calls a Windows API function every time (@jkotas called it "slow" in Enforce scatter/gather file I/O Windows API requirements et. al. #57424 (comment)). Doubt it can change, couldn't find anything online (searching "windows change page size" returns articles about changing the size of the page file, an unrelated thing").
  • UserDomainName/UserInteractive/UserName - I'm not sure about whether they can change in the middle of a program's execution, but at least we could cache identical string objects like I proposed with CurrentDirectory.
  • Version - a cross-platform API, it performs reflection and allocates a new string with the same content every time.

Other properties like OSVersion and ProcessId are already cached. We could cache more unchangable properties as well to improve performance. I might be wrong about some of them, feel free to modify the list accordingly.

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Aug 15, 2021
@filipnavara
Copy link
Member

filipnavara commented Aug 15, 2021

UserDomainName/UserInteractive/UserName - I'm not sure about whether they can change in the middle of a program's execution

I would guess they do change when you do impersonation (eg. WindowsIdentity.Impersonate).

@teo-tsirpanis
Copy link
Contributor Author

Nice catch @filipnavara, do you know whether they can change on Unix?

@jkotas
Copy link
Member

jkotas commented Aug 15, 2021

GetCommandLineArgs which needlessly clones an internal array

The caller can modify the array. The clone is done to make sure that the next caller gets the unmodified content. Removing the clone would be a breaking change.

@teo-tsirpanis
Copy link
Contributor Author

The caller can modify the array.

Indeed, but in get_CommandLine the array is cloned for consumption by an internal function. That's what I meant with "needlessly".

@ghost
Copy link

ghost commented Aug 16, 2021

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

Issue Details

I noticed that the following System.Environment properties (on Windows at least) are not cached and instead perform syscalls and allocations to return a value that does not change over the course of a program's lifetime:

  • CommandLine - partially cached, since it does not always call into native code, but it still every time creates a string and calls the public GetCommandLineArgs which needlessly clones an internal array (that clone could also be eliminated when this property gets initialized).
  • CurrentDirectory - this cannot be entirely cached since it might change by directly calling the Windows APIs but we could check whether the current directory has changed since the last time we got this property and avoid a string allocation if it hasn't, returning a cached string object.
  • MachineName - calls a Windows API function and allocates a new string every time. Documentation says that "This name is established at system startup, when the system reads it from the registry."
  • SystemDirectory - calls a Windows API function and allocates a new string every time. Doubt it can change without reinstalling Windows.
  • SystemPageSize - calls a Windows API function every time (@jkotas called it "slow" in Enforce scatter/gather file I/O Windows API requirements et. al. #57424 (comment)). Doubt it can change, couldn't find anything online (searching "windows change page size" returns articles about changing the size of the page file, an unrelated thing").
  • UserDomainName/UserInteractive/UserName - I'm not sure about whether they can change in the middle of a program's execution, but at least we could cache identical string objects like I proposed with CurrentDirectory.
  • Version - a cross-platform API, it performs reflection and allocates a new string with the same content every time.

Other properties like OSVersion and ProcessId are already cached. We could cache more unchangable properties as well to improve performance. I might be wrong about some of them, feel free to modify the list accordingly.

Author: teo-tsirpanis
Assignees: -
Labels:

area-System.Runtime, untriaged

Milestone: -

@tannergooding tannergooding added needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration and removed untriaged New issue has not been triaged by the area owner labels Aug 16, 2021
@tannergooding tannergooding added this to the Future milestone Aug 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Runtime needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration
Projects
None yet
Development

No branches or pull requests

5 participants