1362225 - Speed up localized message generation

Status: RESOLVED FIXED

(WebExtensions :: General, enhancement)

Description

[Kris Maglione [:kmag]]

A lot of the overhead here is in calculating the locale list, which we can cache, and interpolating substitutions, which we can skip for messages that don't contain a "$".

Comment 1

[Kris Maglione [:kmag]]

Attachment: Bug 1362225 - Speed up i18n.getMessage.

Review commit: https://reviewboard.mozilla.org/r/136340/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/136340/

Comment 2

[Andrew Swan [:aswan]]

Comment on attachment 8864696 [details]
Bug 1362225 - Speed up i18n.getMessage.

https://reviewboard.mozilla.org/r/136340/#review139660

::: toolkit/components/extensions/ExtensionCommon.jsm:1170
(Diff revision 1)
>        cloneScope: null,
>      };
>  
> -    options = Object.assign(defaultOptions, options);
> -
> -    let locales = new Set([this.BUILTIN, options.locale, this.defaultLocale]
> +    let locales = this.availableLocales;
> +    if (options.locale) {
> +      locales = new Set([this.BUILTIN, options.locale, this.defaultLocale]

It looks like you don't need to re-filter all the locales in this case, i.e., this could just be
```
if (this.messages.has(options.locale)) {
  locales.add(options.locale);
}
```

::: toolkit/components/extensions/ExtensionCommon.jsm:1322
(Diff revision 1)
>      return Locale.getLocale().replace(/-/g, "_");
>    },
>  };
>  
> +defineLazyGetter(LocaleData.prototype, "availableLocales", function() {
> +  return new Set([this.BUILTIN, this.selectedLocale, this.defaultLocale]

It doesn't look like selectedLocale was included before but now you've added it?

Comment 3

[Kris Maglione [:kmag]]

Comment on attachment 8864696 [details]
Bug 1362225 - Speed up i18n.getMessage.

https://reviewboard.mozilla.org/r/136340/#review139660

> It looks like you don't need to re-filter all the locales in this case, i.e., this could just be
> ```
> if (this.messages.has(options.locale)) {
>   locales.add(options.locale);
> }
> ```

That would change the ordering, and we'd still need to re-filter. Anyway, this doesn't matter all that much, since it's only used at installation time.

> It doesn't look like selectedLocale was included before but now you've added it?

It was the default value for options.locale. This is only used when no options.locale value is provided.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/65f5ee1f5db8
Speed up i18n.getMessage. r=aswan

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/65f5ee1f5db8