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

[browser][non-icu] HybridGlobalization faster encoding for change case. #85516

Merged
merged 5 commits into from
May 3, 2023

Conversation

ilonatommy
Copy link
Member

@ilonatommy ilonatommy commented Apr 28, 2023

Implements a chunk of web-api based globalization. Is a part of HybridGlobalization feature and contributes to #79989.

Speeds up the decoding process:

measurement time without this PR [ms] time with this PR [ms] time ICU4C [ms] increase vs ICU [times]
String, TextInfo ToLower 2.4686 1.6429 0.4260 3,80
String, TextInfo ToUpper 2.5142 1.0003 0.4432 2,25
String, TextInfo ToTileCase 47.0515 44.8951 14.8432 3,02

An attempt of encoding implementation using TextEncoder was actually slower than existing one, so it did not get into this PR.

measurement time with this PR [ms] time with this PR + TextEncoder [ms]
String, TextInfo ToLower 1.6429ms 3.0513ms
String, TextInfo ToUpper 1.0003ms 3.1530ms
String, TextInfo ToTileCase 44.8951ms 55.6864ms

Relevant discussions:

invariant change case on C# side: #85510 (comment)
TextEncoder usage implications: #85510 (comment)

@ghost
Copy link

ghost commented Apr 28, 2023

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

Implements a chunk of web-api based globalization. Is a part of HybridGlobalization feature and contributes to #79989.

Speeds up the decoding process:

measurement time without this PR [ms] time with this PR [ms] time ICU4C [ms] increase vs ICU [times]
String, TextInfo ToLower 2.4686 1.6429 0.4260 3,80
String, TextInfo ToUpper 2.5142 1.0003 0.4432 2,25
String, TextInfo ToTileCase 47.0515 44.8951 14.8432 3,02

An attempt of encoding implementation using TextEncoder was actually slower than existing one, so it did not get into this PR.

measurement time with this PR [ms] time with this PR + TextEncoder [ms]
String, TextInfo ToLower 1.6429ms 3.0513ms
String, TextInfo ToUpper 1.0003ms 3.1530ms
String, TextInfo ToTileCase 44.8951ms 55.6864ms
Author: ilonatommy
Assignees: ilonatommy
Labels:

arch-wasm, area-System.Globalization

Milestone: -

@ilonatommy ilonatommy merged commit e15f073 into dotnet:main May 3, 2023
ilonatommy added a commit to ilonatommy/runtime that referenced this pull request May 23, 2023
ilonatommy added a commit that referenced this pull request May 24, 2023
* Normalization.

* Missing measurement.

* Trying to fix trimming error.

* Applied @pavelsavara's idea.

* Code review improvement.

* Feedback: standardize the exceptions.

* Revert changes from #85516 to clean the CI.

* feedback
@ghost ghost locked as resolved and limited conversation to collaborators Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants