Closed
Bug 1170072
Opened 7 years ago
Closed 7 years ago
Remove nsCharProps1 if using ICU
Categories
(Core :: Internationalization, defect)
Core
Internationalization
Tracking
()
RESOLVED
FIXED
mozilla43
People
(Reporter: m_kato, Assigned: m_kato)
References
Details
Attachments
(2 files, 1 obsolete file)
5.34 KB,
patch
|
jfkthame
:
review+
|
Details | Diff | Splinter Review |
34.37 KB,
patch
|
jfkthame
:
review+
|
Details | Diff | Splinter Review |
All members of nsCharProps1 can replace with ICU API
Assignee | ||
Comment 1•7 years ago
|
||
WIP. Also, I should add fix to perl script (genUnicodePropertyData.pl) for this.
Assignee | ||
Comment 2•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b2f6ffea4d07
Assignee | ||
Comment 3•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a036f6782c93
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → m_kato
Assignee | ||
Comment 4•7 years ago
|
||
Attachment #8613366 -
Attachment is obsolete: true
Assignee | ||
Comment 5•7 years ago
|
||
Assignee | ||
Comment 6•7 years ago
|
||
Comment on attachment 8655876 [details] [diff] [review] Part 1. Make GetCharProps1 as static function nsCharProps1 can replace with ICU, so I want that GetCharProps1 isn't exported to replace with ICU.
Attachment #8655876 -
Flags: review?(jfkthame)
Assignee | ||
Comment 7•7 years ago
|
||
Comment on attachment 8655878 [details] [diff] [review] Part 2. Use ICU instead of GetCharProps1 if available If ICU is available, we should use ICU API instead of using nsCharProps1
Attachment #8655878 -
Flags: review?(jfkthame)
Comment 8•7 years ago
|
||
Just wondering, have you compared performance at all (e.g. talos tp numbers) to see if this makes any measurable change (either better or worse) compared to our existing code? I think character property lookup is pretty hot code.
Assignee | ||
Comment 9•7 years ago
|
||
talos https://treeherder.mozilla.org/#/jobs?repo=try&revision=eec9b0c85f39
Comment 10•7 years ago
|
||
OK, looks like this isn't going to move the needle perceptibly (either way), which is fine - thanks.
Updated•7 years ago
|
Attachment #8655876 -
Flags: review?(jfkthame) → review+
Comment 11•7 years ago
|
||
Comment on attachment 8655878 [details] [diff] [review] Part 2. Use ICU instead of GetCharProps1 if available Review of attachment 8655878 [details] [diff] [review]: ----------------------------------------------------------------- Looks OK to me. (Please make the commit message a bit more complete - e.g. insert "instead of GetCharProps1 and its supporting data", or something like that.)
Attachment #8655878 -
Flags: review?(jfkthame) → review+
Comment 12•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/eab3103736eb https://hg.mozilla.org/integration/mozilla-inbound/rev/5bc2bf65bcf5
Comment 13•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/eab3103736eb https://hg.mozilla.org/mozilla-central/rev/5bc2bf65bcf5
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox43:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in
before you can comment on or make changes to this bug.
Description
•