Closed Bug 1170072 Opened 4 years ago Closed 4 years ago

Remove nsCharProps1 if using ICU


(Core :: Internationalization, defect)

Not set



Tracking Status
firefox41 --- affected
firefox43 --- fixed


(Reporter: m_kato, Assigned: m_kato)


(Blocks 1 open bug)



(2 files, 1 obsolete file)

All members of nsCharProps1 can replace with ICU API
Attached patch WIP (obsolete) — Splinter Review
WIP.  Also, I should add fix to perl script ( for this.
Blocks: 1184116
Assignee: nobody → m_kato
Attachment #8613366 - Attachment is obsolete: true
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)
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)
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.
OK, looks like this isn't going to move the needle perceptibly (either way), which is fine - thanks.
Attachment #8655876 - Flags: review?(jfkthame) → review+
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+
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.