1296618 - Optimize message formatting

Status: RESOLVED FIXED

(L20n :: JS Library, defect)

Description

[Staś Małolepszy :stas]

formatEntities is an internal method used only in LocalizationObserver.getElementsTranslation.  As evidenced by bug 1289530 we spend quite a lot of time in getElementsTranslation.  I'd like to ry to optimize both methods perf-wise.

Comment 1

[Staś Małolepszy :stas]

I run a few tests using Zibi's l20n-perf-monitor [1].  At first I suspected the slowness to be related to this.iteractive in formatEntities or the destructuring assignment in getElementsTranslation's map() (which tends to be slow as of today).  I can now confirm that formatting takes up to 20 ms which is almost all of what getElementsTranslation takes. 

[1] https://github.com/zbraniecki/gecko-dev/blob/l20n-zibi/toolkit/content/l20n-perf-monitor.js

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Not sure if it's relevant here, but I have seen things like map(), etc. perform slower than a traditional `for (var i < 0; i < length; i++) { ... }` loop, so it might be worth an attempt if you are in a hot code path.

Comment 3

[Staś Małolepszy :stas]

Thanks, :jryans!  I'll experiment

Comment 4

[Staś Małolepszy :stas]

(Accidental Submit.(

I'll experiment with regular for loops (and perhaps for-of) but so far it looks like the most expensive thing is iterating over the NodeList returned from querySelectorAll('[data-l10n-id]'), no matter how.

Comment 5

[Staś Małolepszy :stas]

Most of the perf problems are in format* functions or in the MessageContext class.

I found three major bottlenecks:

* destructuring assignment is slow in hotter loops,

* formatting traits bails out of the simple-string optimization because the entire trait object is passed to format() instead of its <code>val</code>,

* formatting entities with null values and no default traits is slow because we check each trait if it's a default one deep in the generator-enabled code of MessageContext;  we could make this check much earlier.

Comment 6

[Staś Małolepszy :stas]

Attachment: Pull request

This brings formatting (keysFromContext) from 20ms down to 9ms. I'll run a few more experiments before asking for review.

Comment 7

[Staś Małolepszy :stas]

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

Comment 8

[Staś Małolepszy :stas]

Perfherder numbers have low confidence due to the small sample but they're consistent with what I'm seeing locally:

https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=ccade2a11d17&newProject=try&newRevision=c57005f7a569&framework=1&filter=tpaint&showOnlyImportant=0

There seems to be a small improvement in tpaint numbers.  I think this is ready to land.

Comment 9

[Staś Małolepszy :stas]

Attachment: Bug 1296618 - Part 1: Format trait.val directly.

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

Comment 10

[Staś Małolepszy :stas]

Attachment: Bug 1296618 - Part 2: Optimize entities with null values and no default traits.

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

Comment 11

[Staś Małolepszy :stas]

Attachment: Bug 1296618 - Part 3: Remove destructuring of [id, args] arrays.

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

Comment 12

[Staś Małolepszy :stas]

Attachment: Bug 1296618 - Part 4: Remove destructuring of [value, errors] arrays.

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

Comment 13

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8785010 [details]
Bug 1296618 - Part 4: Remove destructuring of [value, errors] arrays.

https://reviewboard.mozilla.org/r/74318/#review72310

Comment 14

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8785009 [details]
Bug 1296618 - Part 3: Remove destructuring of [id, args] arrays.

https://reviewboard.mozilla.org/r/74316/#review72308

Comment 15

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8785008 [details]
Bug 1296618 - Part 2: Optimize entities with null values and no default traits.

https://reviewboard.mozilla.org/r/74314/#review72306

Comment 16

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8785007 [details]
Bug 1296618 - Part 1: Format trait.val directly.

https://reviewboard.mozilla.org/r/74312/#review72304

Comment 17

[Staś Małolepszy :stas]

https://github.com/l20n/l20n.js/commit/15748fa0310bbf5c4166d76c047f520c8705857d

Comment 18

[Staś Małolepszy :stas]

https://hg.mozilla.org/projects/larch/rev/aa591eaf759c385e131bec79f0262976d195d209
Bug 1296618 - Part 1: Format trait.val directly. r=gandalf

https://hg.mozilla.org/projects/larch/rev/38d37db7b551c7ecfa286178001b42502b9bc3bc
Bug 1296618 - Part 2: Optimize entities with null values and no default traits. r=gandalf

https://hg.mozilla.org/projects/larch/rev/5d24f47b89d6d130a18d0b1a883c1e8c35c10f67
Bug 1296618 - Part 3: Remove destructuring of [id, args] arrays. r=gandalf

https://hg.mozilla.org/projects/larch/rev/468244986b20a0bdf22a5e36b55ae43ac09599c8
Bug 1296618 - Part 4: Remove destructuring of [value, errors] arrays. r=gandalf