1170072 - Remove nsCharProps1 if using ICU

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[Makoto Kato [:m_kato] (PTO until 5/5)]

All members of nsCharProps1 can replace with ICU API

Comment 1

[Makoto Kato [:m_kato] (PTO until 5/5)]

Attachment: WIP

WIP.  Also, I should add fix to perl script (genUnicodePropertyData.pl) for this.

Comment 2

[Makoto Kato [:m_kato] (PTO until 5/5)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b2f6ffea4d07

Comment 3

[Makoto Kato [:m_kato] (PTO until 5/5)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a036f6782c93

Comment 4

[Makoto Kato [:m_kato] (PTO until 5/5)]

Attachment: Part 1. Make GetCharProps1 as static function

Comment 5

[Makoto Kato [:m_kato] (PTO until 5/5)]

Attachment: Part 2. Use ICU instead of GetCharProps1 if available

Comment 6

[Makoto Kato [:m_kato] (PTO until 5/5)]

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.

Comment 7

[Makoto Kato [:m_kato] (PTO until 5/5)]

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

Comment 8

[Jonathan Kew (: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.

Comment 9

[Makoto Kato [:m_kato] (PTO until 5/5)]

talos
https://treeherder.mozilla.org/#/jobs?repo=try&revision=eec9b0c85f39

Comment 10

[Jonathan Kew (:jfkthame)]

OK, looks like this isn't going to move the needle perceptibly (either way), which is fine - thanks.

Comment 11

[Jonathan Kew (:jfkthame)]

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.)

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/eab3103736eb
https://hg.mozilla.org/integration/mozilla-inbound/rev/5bc2bf65bcf5

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/eab3103736eb
https://hg.mozilla.org/mozilla-central/rev/5bc2bf65bcf5