-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Reduce unsafe usage in TextInfo.cs #122116
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
Reduce unsafe usage in TextInfo.cs #122116
Conversation
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.
Pull request overview
This PR successfully reduces unsafe code usage in TextInfo.cs by refactoring the ToLowerAsciiInvariant(string) method to use modern, safe C# patterns. The method is used internally for caching culture names (typically short strings like "aa-BB") and is not performance-critical.
Key Changes
- Removed
unsafemodifier from the method signature - Replaced pointer-based operations (
fixed,FastAllocateString) withstring.CreateAPI - Simplified the implementation using span-based iteration with
foreach (ref char c in span)
src/libraries/System.Private.CoreLib/src/System/Globalization/TextInfo.cs
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Globalization/TextInfo.cs
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Globalization/TextInfo.cs
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Globalization/TextInfo.cs
Show resolved
Hide resolved
tarekgh
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!
|
/ba-g AOT timeouts |
@MihaZupan NativeAOT failures were not all timeouts:
It is a real failure: I suspect that this regression was introduced by adding a dependency on |
|
MichalStrehovsky/rt-sz#203 confirmed this change introduced 200kB - 300kB regression in hello world NAOT binary size. |

Getting rid of some low-hanging fruit.
The only place this helper is used is from the
CultureInfoctor, via:runtime/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.cs
Lines 2286 to 2289 in c8675d9
Representative call site (see line 1033):
runtime/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureInfo.cs
Lines 1029 to 1033 in c8675d9
This is: (a) not a hot code path; and (b) expected to contain very small strings, typically of the form
aa-BB. Seems like it's an ideal candidate for simplifying the code.