WebExtensions tabs.query is much slower than gBrowser.tabs

RESOLVED FIXED in Firefox 52

Status

RESOLVED FIXED
2 years ago
9 months ago

People

(Reporter: u462496, Assigned: kmag)

Tracking

unspecified
mozilla53

Firefox Tracking Flags

(firefox51 wontfix, firefox52 fixed, firefox53 fixed)

Details

Attachments

(5 attachments)

Comment hidden (empty)
(Reporter)

Comment 1

2 years ago
(Reporter)

Comment 2

2 years ago
(Reporter)

Comment 3

2 years ago
str
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.

Updated

2 years ago
Whiteboard: investigation
(Assignee)

Updated

2 years ago
Assignee: nobody → kmaglione+bmo
Component: WebExtensions: Untriaged → WebExtensions: Frontend
Whiteboard: investigation
Comment hidden (mozreview-request)
(Assignee)

Comment 6

2 years ago
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

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/31943df12a3a
https://hg.mozilla.org/mozilla-central/rev/7f813afa9a8c
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Attachment #8820023 - Flags: review?(aswan)
Attachment #8820024 - Flags: review?(aswan)
(Assignee)

Comment 8

2 years ago
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.
Attachment #8820023 - Flags: approval-mozilla-beta?
Attachment #8820023 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 9

2 years ago
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.
Attachment #8820024 - Flags: approval-mozilla-beta?
Attachment #8820024 - Flags: approval-mozilla-aurora?
(Reporter)

Comment 10

2 years ago
(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 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.
Attachment #8820023 - Flags: approval-mozilla-beta?
Attachment #8820023 - Flags: approval-mozilla-beta+
Attachment #8820023 - Flags: approval-mozilla-aurora?
Attachment #8820023 - Flags: approval-mozilla-aurora+
Andrei can your team test this fix in beta 11 next week? Thanks!
Flags: needinfo?(andrei.vaida)
Part 2 is hitting merge conflicts due to bug 1319567. Need an uplift request there or a rebased patch.
status-firefox51: --- → affected
status-firefox52: --- → affected
Flags: needinfo?(kmaglione+bmo)
(Assignee)

Comment 14

2 years ago
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.
Flags: needinfo?(kmaglione+bmo)
Attachment #8820024 - Flags: approval-mozilla-beta?
Attachment #8820024 - Flags: approval-mozilla-beta+
Attachment #8820024 - Flags: approval-mozilla-aurora?
Attachment #8820024 - Flags: approval-mozilla-aurora+
Please go ahead with part 2 as well! I was distracted yesterday and missed twiddling the flag on part 2.
This'll need rebasing for Beta too, unfortunately :(
Flags: needinfo?(kmaglione+bmo)
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?
Too late for 51. Mark 51 won't fix.
status-firefox51: affected → wontfix
Flags: needinfo?(andrei.vaida)
(Assignee)

Updated

a year ago
Flags: needinfo?(kmaglione+bmo)

Updated

9 months ago
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.