1355424 - Consider making updateCurrentBrowser interruptible

Status: RESOLVED WORKSFORME

(Firefox :: Tabbed Browser, enhancement, P4)

Description

[Florian Quèze [:florian]]

In bug 1349822 we are discussing removing from updateCurrentBrowser the things that may be slower than they need to be, or that may not be useful (eg. add-on sdk overhead), but I think at some point we'll run out of things to optimize there.

I'm under the impression that the slowness of updateCurrentBrowser is the most user-perceptible when we call it several times in a row, for example when the user presses Cmd+w several times. When this happens, I think we could probably skip most of the work updateCurrentBrowser does as soon as we know we are about to change the current browser again. To do this, I think we should make updateCurrentBrowser let some events be processed while it's running, and return early if it has been called again due to a new user event.

In pseudo code, I'm thinking something like this:

var gUpdateId = 0;
async function updateCurrentBrowser() {
  let id = ++gUpdateId;

  let instructions = [
   () => { stuff(); },
...
   () => { stuff(); },
  ];
  for (let instruction of instructions) {
    if (gUpdateId != id)
      break;
    instruction();
    await executeSoon();
  }
}

This isn't a fully thought through idea yet, but I'm filing a bug so that we don't forget to evaluate it once we are done optimizing away the slow things we don't need.

One tricky thing we should pay attention to is that any keyboard event that isn't related to switching the current tab should be processed after the focus has reached the correct element. This likely means that we would need to add an additional key event listener, and force updateCurrentBrowser to finish synchronously when receiving an event that needs to be processed.

Comment 1

[:Gijs (he/him)]

I did some quick profiling and from a quick look it does not appear like the impact of updateCurrentBrowser is super significant anymore at this point. Even when holding down ctrl-tab, for about ~2000 samples, only about 150 were from updateCurrentBrowser or code underneath it. An individual tabswitch shows anywhere between 0 and 6 samples (admittedly on my fast mbp).

I'm going to suggest most of this has been addressed by AsyncTabSwitcher, and where it hasn't, we'd be better served by more specific, up-to-date reports, so I'm going to close this out.