Cache UPluralRules and UNumberFormat for Intl.PluralRules instances

RESOLVED FIXED in Firefox 60

Status

()

enhancement
P3
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: anba, Assigned: anba)

Tracking

Trunk
mozilla60
Points:
---

Firefox Tracking Flags

(firefox60 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

a year ago
Caching UPluralRules and UNumberFormat avoids creating new ones for every call to Intl.PluralRules.prototype.select.
(Assignee)

Comment 1

a year ago
Adds caching for UPluralRules and UNumberFormat to Intl.PluralRules.

I've also modified intl_GetPluralCategories() to compute the available categories from an Intl.PluralRules instance instead of locale and type string arguments. This allows intl_GetPluralCategories() to use the UPluralRules caching, too. Furthermore I've removed to call to intl_GetPluralCategories() in resolvePluralRulesInternals() and instead moved it to Intl_PluralRules_resolvedOptions(). That way we only need to compute the available categories when Intl_PluralRules_resolvedOptions() is actually called.
Attachment #8947911 - Flags: review?(gandalf)
Comment on attachment 8947911 [details] [diff] [review]
bug1435306.patch

Review of attachment 8947911 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm! Thanks!
Attachment #8947911 - Flags: review?(gandalf) → review+

Comment 4

a year ago
Pushed by nbeleuzu@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a3d21e203519
Cache UPluralRules and UNumberFormat for Intl.PluralRules instances. r=gandalf
Keywords: checkin-needed

Comment 5

a year ago
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cefac2e7eb0c
followup - Fix rooting hazard. r=red CLOSED TREE
anba, the rooting hazard complained because it thinks uplrules_close can GC.

The sheriffs asked about this and I pushed a follow-up fix, restructuring the code a little. Feel free to land a better fix.
(In reply to Jan de Mooij [:jandem] from comment #6)
> the rooting hazard complained

Ugh, rooting analysis, of course.

Comment 8

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a3d21e203519
https://hg.mozilla.org/mozilla-central/rev/cefac2e7eb0c
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
(Assignee)

Comment 9

a year ago
(In reply to Jan de Mooij [:jandem] from comment #6)
> anba, the rooting hazard complained because it thinks uplrules_close can GC.
> 
> The sheriffs asked about this and I pushed a follow-up fix, restructuring
> the code a little. Feel free to land a better fix.

The change looks good to me! Thanks for fixing it.
You need to log in before you can comment on or make changes to this bug.