1808719 - Optimize consumers of slow gBrowser.visibleTabs

Status: NEW

(Firefox :: Tabbed Browser, task, P3)

Description

[Oriol Brufau [:Oriol]]

https://searchfox.org/mozilla-central/rev/d6a131ceb435c03ccab2592578f6e2ebf12c1644/browser/base/content/tabbrowser.js#290-298

    get visibleTabs() {
      if (!this._visibleTabs) {
        this._visibleTabs = Array.prototype.filter.call(
          this.tabs,
          tab => !tab.hidden && !tab.closing
        );
      }
      return this._visibleTabs;
    },

This getter is used very frequently, and it's slow when there are lots of tabs. The result is cached, but that won't help when e.g. removing a thousand of tabs, since each removal will invalidate the cache and do the work again.

It doesn't make sense to do that work just to e.g. check gBrowser.visibleTabs.length === 1.

Comment 1

[Dão Gottwald [:dao]]

How much of a problem do you think this still is? Bug 1837174 removes another crucial user of this, btw.

Comment 2

[Dão Gottwald [:dao]]

For consumers that have a tab and want to check if it's the only one, a low-effort way to provide that information, without e.g. implementing a visible tab counter, could be to add an isOnlyVisibleTab property that does something like this:

return !this.hidden &&
  !this.closing &&
  this.selected &&
  this.matches(":nth-child(1 of :not(.tabbrowser-tab[hidden])):nth-last-child(1 of .tabbrowser-tab:not([hidden]))");

I'm not entirely sure it would be a net-win for performance though. If something accesses visibleTabs shortly after for other purposes, we'd have to rebuild it anyway.

Comment 3

[Oriol Brufau [:Oriol]]

I will have to go back and test perf with some draft patches that I had. Instead of matches() I was thinking of a lazy computation of visibleTabs: instead of building the full list, it would only cache as much as needed (so it would stop iterating at the 2nd visible tab if we only need to check if there is a single one).