1322869 - WebExtensions tabs.query is much slower than gBrowser.tabs

Status: RESOLVED FIXED

(WebExtensions :: Frontend, defect)

Comment 1

[u462496]

Attachment: [e-mail]

Comment 2

[u462496]

Attachment: WE_tabs_query_demo.zip

Comment 3

[u462496]

According to benchmarking I've done, iterating tabs through WE tabs.query is about 22 times slower than gBrowser.tabs.  I've uploaded a XUL extension and WE extension (in zipped file) for demonstration.

Install both extensions, install the toolbar button for the XUL extension ("XL1" icon).

Open browser console.

Tools > "Set Browser to Predefined State" > ... and set browser state to 100, 300 or 500 tabs in single window.

Clicking "XL1" toolbar button will iterate over `gBrowser.tabs`, accessing `tab.label` and `tab.getAttribute("image")` for each tab.  Results will be displayed in browser console log.

Clicking "WE2" toolbar button will iterate over `browser.tabs.query()`, accessing `tab.title` and `tab.favIconUrl` for each tab.  Results will be displayed in browser console log.

Results displayed in browser console will be the et of the current test, and the average et of the last 5 tests.

Comment 4

[Kris Maglione [:kmag]]

Attachment: Bug 1322869: Part 1 - Apply window filters per-window rather than per-tab.

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

Comment 5

[Kris Maglione [:kmag]]

Attachment: Bug 1322869: Part 2 - Memoize some relatively expensive-to-compute tab attributes.

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

Comment 6

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/31943df12a3aafc9846b852da24a676fc18505fc
Bug 1322869: Part 1 - Apply window filters per-window rather than per-tab. r=bsilverberg

https://hg.mozilla.org/integration/mozilla-inbound/rev/7f813afa9a8c9deda34b19cb31bedf433f8d8e3c
Bug 1322869: Part 2 - Memoize some relatively expensive-to-compute tab attributes. r=bsilverberg

Comment 7

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

https://hg.mozilla.org/mozilla-central/rev/31943df12a3a
https://hg.mozilla.org/mozilla-central/rev/7f813afa9a8c

Comment 8

[Kris Maglione [:kmag]]

Comment on attachment 8820023 [details]
Bug 1322869: Part 1 - Apply window filters per-window rather than per-tab.

Approval Request Comment
[Feature/Bug causing the regression]: N/A
[User impact if declined]: This issue could cause noticeable performance problems for users of WebExtensions with large numbers of tabs open.
[Is this code covered by automated tests?]: The behavior of this function is covered by extensive tests. There are no specific tests for its performance.
[Has the fix been verified in Nightly?]: No.
[Needs manual test from QE? If yes, steps to reproduce]: The reporter provided steps to reproduce in comment 3.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: Very low-risk.
[Why is the change risky/not risky?]: This patch simply slightly rearranges some filtering code so that window filters are not needlessly reevaluated for each tab.
[String changes made/needed]: None.

Comment 9

[Kris Maglione [:kmag]]

Comment on attachment 8820024 [details]
Bug 1322869: Part 2 - Memoize some relatively expensive-to-compute tab attributes.

Approval Request Comment
[Feature/Bug causing the regression]: N/A
[User impact if declined]: This issue could cause noticeable performance problems for users of WebExtensions with large numbers of tabs open.
[Is this code covered by automated tests?]: The behavior of this function is covered by extensive tests. There are no specific tests for its performance.
[Has the fix been verified in Nightly?]: No.
[Needs manual test from QE? If yes, steps to reproduce]: The reporter provided steps to reproduce in comment 3.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: Low-risk.
[Why is the change risky/not risky?]: This patch simply caches the values of two tab attributes which are no expected to change throughout the life of the browser.
[String changes made/needed]: None.

Comment 10

[u462496]

(In reply to Kris Maglione [:kmag] from comment #9)
> [Has the fix been verified in Nightly?]: No.

Thanks for your work, Kris.

Nightly 53.0a1 (2016-12-24)

300 tabs:
browser.tabs.query : 50ms avg
gBrowser.tabs : 5ms avg

500 tabs:
browser.tabs.query : 72ms avg
gBrowser.tabs : 6ms avg

An improvement, though still about 10 to 12 times slower than gBrowser.tabs.

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8820023 [details]
Bug 1322869: Part 1 - Apply window filters per-window rather than per-tab.

Perf improvement for web extensions, let's uplift this to aurora and beta.
It should make it into the beta 11 build next Monday.

Comment 12

[Liz Henry (:lizzard) (relman/hg->git project)]

Andrei can your team test this fix in beta 11 next week? Thanks!

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Part 2 is hitting merge conflicts due to bug 1319567. Need an uplift request there or a rebased patch.

Comment 14

[Kris Maglione [:kmag]]

Part 2 doesn't have uplift approval yet. Either way, part 1 is by far the bigger win, so it should probably be uplifted on its own.

Comment 15

[Liz Henry (:lizzard) (relman/hg->git project)]

Please go ahead with part 2 as well! I was distracted yesterday and missed twiddling the flag on part 2.

Comment 16

[Kris Maglione [:kmag]]

Attachment: Part 2, rebased for Aurora

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1cb53f202c0c
https://hg.mozilla.org/releases/mozilla-aurora/rev/467835517dc4

Comment 18

[Ryan VanderMeulen [:RyanVM]]

This'll need rebasing for Beta too, unfortunately :(

Comment 19

[Vasilica Tamas, QA [:vtamas]]

Performed exploratory testing around this issue on Windows 10 64-bit and Ubuntu 16.04 32-bit and noted the results: 
 - Affected version - Firefox 53.0a1 (2016-12-10) 
     * Windows: http://pastebin.com/1hDrFPiS
     * Ubuntu: http://pastebin.com/Ad5ssdVj

 - Fixed Nightly - Firefox 53.0a1 (2017-01-02)
     * Windows: http://pastebin.com/kmSUp7TD
     * Ubuntu: http://pastebin.com/warRv1ga

 - Fixed Dev Edition - Firefox 52.0a2 (2017-01-03) 
     * Windows: http://pastebin.com/teQtFXsQ
     * Ubuntu: http://pastebin.com/bQu5HB6U

As Kevin Jones mentioned in Comment 10, there can be seen a small improvement but is still a big difference between gBrowser.tabs and browser.tabs.query performance. 

Kris, are these the expected values?

Comment 20

[Gerry Chang [:gchang]]

Too late for 51. Mark 51 won't fix.