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

Globalization API optimization opportunities #9975

Closed
jkotas opened this issue Mar 20, 2018 · 5 comments
Closed

Globalization API optimization opportunities #9975

jkotas opened this issue Mar 20, 2018 · 5 comments
Labels
area-System.Globalization backlog-cleanup-candidate An inactive issue that has been marked for automated closure. enhancement Product code improvement that does NOT require public API changes/additions help wanted [up-for-grabs] Good issue for external contributors no-recent-activity tenet-performance Performance related issue

Comments

@jkotas
Copy link
Member

jkotas commented Mar 20, 2018

From dotnet/coreclr#17029 (comment)

  • Speculatively allocate array for LCMapStringEx / GetSortKey during case-insensitive hashing
  • Do not use SafeHandle for sort handle on Unix (we are not using SafeHandle for this on Windows either)
  • Use arguments that do not require marshaling in globalization API signature (pointers instead of strings, etc.)
@tarekgh
Copy link
Member

tarekgh commented Mar 20, 2018

Do not use SafeHandle for sort handle on Unix (we are not using SafeHandle for this on Windows either)

I think we still need the safe handle on Linux. sorting handle on Linux needs to be closed when done using it which is different than Windows.

https://github.com/dotnet/coreclr/blob/57ee22e6086af5dfc06651dc002e24a71c3999ee/src/mscorlib/shared/Interop/Unix/System.Globalization.Native/Interop.Collation.cs#L75

@jkotas
Copy link
Member Author

jkotas commented Mar 20, 2018

CultureInfos are cached in a global hashtable and pretty much never disposed. Two interlocked operations for every globalized string operations is a pretty steep price to pay to handle the very rare case when the CultureInfos are disposed. There are other ways to deal with this than safe handle.

@tarekgh
Copy link
Member

tarekgh commented Mar 20, 2018

CultureInfos are cached in a global hashtable and pretty much never disposed

It is possible to get disposed. Also, it is possible creating different CompareInfo objects which can get collected and leak sort handles

https://github.com/dotnet/coreclr/blob/a4af006ff55af59ac5d28f0b4faffdc1806dc89a/src/mscorlib/src/System/Globalization/CultureInfo.cs#L707

new CultureInfo("en-US", false).CompareInfo

We may look at caching the sort handles in Linux too.

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 26, 2020
@tarekgh tarekgh removed the untriaged New issue has not been triaged by the area owner label Jun 17, 2020
Copy link
Contributor

Due to lack of recent activity, this issue has been marked as a candidate for backlog cleanup. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will undo this process.

This process is part of our issue cleanup automation.

@dotnet-policy-service dotnet-policy-service bot added backlog-cleanup-candidate An inactive issue that has been marked for automated closure. no-recent-activity labels Dec 17, 2024
Copy link
Contributor

This issue will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the issue, but please note that the issue will be locked if it remains inactive for another 30 days.

@dotnet-policy-service dotnet-policy-service bot removed this from the Future milestone Dec 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Globalization backlog-cleanup-candidate An inactive issue that has been marked for automated closure. enhancement Product code improvement that does NOT require public API changes/additions help wanted [up-for-grabs] Good issue for external contributors no-recent-activity tenet-performance Performance related issue
Projects
None yet
Development

No branches or pull requests

4 participants