Closed Bug 1435306 Opened 2 years ago Closed 2 years ago

Cache UPluralRules and UNumberFormat for Intl.PluralRules instances

Categories

(Core :: JavaScript: Internationalization API, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: anba, Assigned: anba)

Details

Attachments

(1 file)

Caching UPluralRules and UNumberFormat avoids creating new ones for every call to Intl.PluralRules.prototype.select.
Attached patch bug1435306.patchSplinter Review
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+
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
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.
https://hg.mozilla.org/mozilla-central/rev/a3d21e203519
https://hg.mozilla.org/mozilla-central/rev/cefac2e7eb0c
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
(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.