1524787 - Optimize incremental filtering and work around bug 1480477 in the new "about:config" page

Status: RESOLVED FIXED

(Toolkit :: Preferences, enhancement, P3)

Description

[:Paolo Amadini]

Per bug 1523028 comment 44, we can improve performance either by changing how DOM nodes are removed or by avoiding ":nth-child". The latter could involve a design change or marking odd rows at table update time.

Comment 1

[:Paolo Amadini]

Attachment: Bug 1524787 - Optimize incremental filtering and work around bug 1480477 in the new "about:config" page. r=mconley

Comment 2

[:Paolo Amadini]

Attachment: Bug 1524787 - Part 1 - Reduce calls to getElementById. r=mconley

Comment 3

[:Paolo Amadini]

Attachment: Bug 1524787 - Part 2 - Use an expando property instead of a WeakMap. r=mconley

Depends on D19511

Comment 4

[:Paolo Amadini]

Attachment: Bug 1524787 - Part 3 - Only set the class name once when the state changes. r=mconley

Depends on D19512

Comment 5

[:Paolo Amadini]

Attachment: Bug 1524787 - Part 4 - Sort the preferences list only when it changes. r=mconley

Depends on D19513

Comment 6

[:Paolo Amadini]

Attachment: Bug 1524787 - Part 5 - Optimize incremental filtering and work around bug 1480477 in the new "about:config" page. r=mconley

Depends on D19514

Comment 7

[:Paolo Amadini]

Attachment: Bug 1524787 - Part 6 - Back out part 4 for testing.

Depends on D19515

Comment 8

[:Paolo Amadini]

Attachment: Bug 1524787 - Part 7 - Benchmark.

Depends on D19516

Comment 9

[:Paolo Amadini]

Here are some rough benchmark results. They are quite noisy, and may differ by a few milliseconds at each run.

I think the improvements we can notice with good confidence are for incremental filtering from Part 4 and Part 5:

  Current:        {bro:23.3, brow:17.8, brows:17.5, browse:17.3, browser:17.2}
  Part 1+2:       {bro:22.9, brow:17.7, brows:17.2, browse:17.1, browser:16.8}
  Part 1+2+3:     {bro:21.3, brow:17.4, brows:17.9, browse:17.6, browser:17.3}
  Part 1+2+3+4:   {bro:21.1, brow:15.3, brows:15.5, browse:15.5, browser:14.8}
  Part 1+2+3+5:   {bro:22.0, brow: 9.6, brows:10.5, browse: 9.9, browser:10.7}
  Part 1+2+3+4+5: {bro:20.4, brow: 8.6, brows: 8.6, browse: 8.8, browser: 9.3}

Comment 10

[:Paolo Amadini]

Mike, Brian, see comment 9, I believe you weren't following the bug even though you reviewed the previous patch.

Comment 11

[:Paolo Amadini]

Since I already got r+ on the combined patch, I'm converting the reviews on the individual patches to a needinfo. We should determine if we still need this to support the perceived performance updates in bug 1527395, or if we can avoid the extra complexity.

Comment 12

[Mike Conley (:mconley) (:⚙️)]

It looks like 2 and 3 and arguably 1 don't end up buying us much here.

4 and 5 seem to be a pretty clear improvement.

Let's try just landing those for now.

Comment 13

[:Paolo Amadini]

Sorry for being unclear, part 2 and 3 are required to avoid regressions in part 5. This is because we try to change the class more often and in different combinations, and for each element in the table we would access the WeakMap, which is apparently slightly slower than using expando properties in this situation.

Comment 14

[Mike Conley (:mconley) (:⚙️)]

Huh, I see. Okay.

Well, I guess better land the stack then. Thanks.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2660bdf28535
Part 1 - Reduce calls to getElementById. r=mconley
https://hg.mozilla.org/integration/mozilla-inbound/rev/619f1112ed34
Part 2 - Use an expando property instead of a WeakMap. r=mconley
https://hg.mozilla.org/integration/mozilla-inbound/rev/158ab30e6b96
Part 3 - Only set the class name once when the state changes. r=mconley
https://hg.mozilla.org/integration/mozilla-inbound/rev/c48000b1845f
Part 4 - Sort the preferences list only when it changes. r=mconley
https://hg.mozilla.org/integration/mozilla-inbound/rev/a1a6fbcf6f35
Part 5 - Optimize incremental filtering and work around bug 1480477 in the new "about:config" page. r=mconley

Comment 16

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/2660bdf28535
https://hg.mozilla.org/mozilla-central/rev/619f1112ed34
https://hg.mozilla.org/mozilla-central/rev/158ab30e6b96
https://hg.mozilla.org/mozilla-central/rev/c48000b1845f
https://hg.mozilla.org/mozilla-central/rev/a1a6fbcf6f35