piroor / multipletab

Multiple Tab Handler, Provides feature to close multiple tabs.
http://piro.sakura.ne.jp/xul/multipletab/
Other
81 stars 23 forks source link

Poor performance when using shift-click to select tabs #221

Closed Lej77 closed 5 years ago

Lej77 commented 5 years ago

Short description

I have several thousand tabs open in Firefox (most of them unloaded) and it can take several seconds when I shift-click on a tab to select the tabs between the current tab and the clicked tab. This is true even if I clicked on the tab right next to the current one so that only two tabs is selected in total. The performance is fine in windows that doesn't have many tabs open, even if the total Firefox session has many tabs.

Selecting tabs using ctrl-click or by holding the left mouse button and dragging works without any lag. Also if I disable MTH and use TST for multi-selecting tabs then it works really fast as well. So it seems that it is only the shift-click operation that has bad performance.

Steps to reproduce

  1. Start Firefox with clean profile.
  2. Install MTH.
  3. Install TST.
  4. Open 2000 tabs.
  5. Shift-click to select multiple tabs. (can be only two tabs in total.)

Expected result

The selection should be fast.

Actual result

The selection takes several seconds.

Environment

piroor commented 5 years ago

Performance profile with TST only, without MTH profiles.zip

Lej77 commented 5 years ago

Performance for my session (TST and MTH installed) when using shift click to select one tab and another when selecting about 15 tabs: my profiles.zip

Lej77 commented 5 years ago

Performance in my session for a window with few tabs (no performance problems): fast profiles.zip

piroor commented 5 years ago

I've realized that there are some bottlenecks. The largest one for me is: updating of group tabs (dummy tabs).

Lej77 commented 5 years ago

One simple solution is to just let tree style tab handle selection and not do any selection work in MTH since that seems to work fine.

piroor commented 5 years ago

After some optimizations, bottlenecks in TST-side have been reduced.

Lej77 commented 5 years ago

I download MTH 2.2.5 and added some debug logging to the DragSelection.onClick function. From the logged info its clear that the performance issues are from TST's code. For most operations MTH's code completes within 100 milliseconds. Since the selection issue takes several seconds that time must be spent in TST's code.

I have not checked how your optimizations affect things so hopefully this issue is already fixed but if not then this might be helpful.

Logged info when shift clicking a tab next to the current tab:

Count:  0   - Total Elapsed:  0     - Since Last:  0    -  start
Count:  1   - Total Elapsed:  1     - Since Last:  1    -  got selection
Count:  2   - Total Elapsed:  4     - Since Last:  3    -  checked if clicked tab is selected
Count:  3   - Total Elapsed:  9     - Since Last:  5    -  got last active tab
Count:  4   - Total Elapsed:  10    - Since Last:  1    -  got affected tabs
Count:  5   - Total Elapsed:  131   - Since Last:  121  -  got all tabs in window
Count:  6   - Total Elapsed:  132   - Since Last:  1    -  got tabs between clicked tabs
Count:  7   - Total Elapsed:  138   - Since Last:  6    -  selection updated 
Count:  8   - Total Elapsed:  138   - Since Last:  0    -  active tab updated to be in selection

Logs in code:


class DebugTimer {
  constructor() {
    let time = Date.now();
    Object.assign(this, {
      counter: 0,
      startTime: time,
      lastTime: time,
    });
  }

  log(...args) {
    try {
      let time = Date.now();

      if (args.length > 0) {
        args.unshift("\t- ");
      }

      console.log(new Date(time).toLocaleTimeString(), "\t- Count: ", this.counter++, "\t- Total Elapsed: ", time - this.startTime, '\t- Since Last: ', time - this.lastTime, ...args);
      this.lastTime = time; 
    } catch (error) {
      console.error('Failed to log time\n', error);
    }
  }
}

export async function onClick(message) {
  if (message.button != 0)
    return false;

  let debugTimer = new DebugTimer();

  debugTimer.log("start");
  mDragSelection.selection = Selections.get(message.window);
  debugTimer.log("got selection");

  let selected = false;
  {
    if (message.tab.states)
      selected = message.tab.states.indexOf('selected') > -1;
    else
      selected = !!mDragSelection.selection.contains(message.tab.id);
  }
  debugTimer.log("checked if clicked tab is selected");

  const ctrlKeyPressed = message.ctrlKey || (message.metaKey && /^Mac/i.test(navigator.platform));
  if (!ctrlKeyPressed && !message.shiftKey) {
    debugTimer.log("no modifier key");
    if (!selected) {
      mDragSelection.selection.clear({
        states: ['selected', 'ready-to-close']
      });
    }
    gInSelectionSession = false;
    mDragSelection.selection.setLastClickedTab(null);
    return;
  }

  const lastActiveTab = message.lastActiveTab || (await browser.tabs.query({
    active:   true,
    windowId: message.window
  }))[0];
  if (lastActiveTab)
    TabIdFixer.fixTab(lastActiveTab);

  debugTimer.log("got last active tab");

  let tabs = retrieveTargetTabs(message.tab);
  debugTimer.log("got affected tabs");
  if (message.shiftKey) {
    // select the clicked tab and tabs between last activated tab
    const window = await browser.windows.get(message.window, { populate: true });
    debugTimer.log("got all tabs in window");
    const betweenTabs = getTabsBetween(mDragSelection.selection.getLastClickedTab() || lastActiveTab, message.tab, window.tabs);
    debugTimer.log("got tabs between clicked tabs");
    tabs = tabs.concat(betweenTabs);
    tabs.push(mDragSelection.selection.getLastClickedTab() || lastActiveTab);
    const selectedTabIds = tabs.map(tab => tab.id);
    if (!ctrlKeyPressed)
      mDragSelection.selection.set(window.tabs.filter(tab => selectedTabIds.indexOf(tab.id) < 0), false, {
        globalHighlight: false
      });
    mDragSelection.selection.set(tabs, true, {
      globalHighlight: false
    });
    debugTimer.log("selection updated");
    gInSelectionSession = true;
    // Selection must include the active tab. This is the standard behavior on Firefox 62 and later.
    const newSelectedTabIds = mDragSelection.selection.getSelectedTabIds();
    if (newSelectedTabIds.length > 0 && !newSelectedTabIds.includes(lastActiveTab.id))
      browser.tabs.update(mDragSelection.selection.getLastClickedTab() ? mDragSelection.selection.getLastClickedTab().id : newSelectedTabIds[0], { active: true });
    debugTimer.log("active tab updated to be in selection");
    return true;
  }
  else if (ctrlKeyPressed) {
    // toggle selection of the tab and all collapsed descendants
    if (message.tab.id != lastActiveTab.id ||
        !gInSelectionSession) {
      mDragSelection.selection.set(lastActiveTab, true, {
        globalHighlight: false
      });
      debugTimer.log("clicked tab's selection changed");
      if (configs.enableIntegrationWithTST)
        await setSelectedStateToCollapsedDescendants(lastActiveTab, true);
      debugTimer.log("clicked tab's descendants selection changed");
    }
    mDragSelection.selection.set(tabs, !selected, {
      globalHighlight: false
    });
    debugTimer.log("selection updated");
    // Selection must include the active tab. This is the standard behavior on Firefox 62 and later.
    const selectedTabIds = mDragSelection.selection.getSelectedTabIds();
    if (selectedTabIds.length > 0 && !selectedTabIds.includes(lastActiveTab.id))
      browser.tabs.update(selectedTabIds[0], { active: true });
    gInSelectionSession = true;
    mDragSelection.selection.setLastClickedTab(message.tab);
    debugTimer.log("active tab updated to be in selection");
    return true;
  }
  return false;
}
Lej77 commented 5 years ago

@piroor I tried the new Tree Style Tab version 2.7.0 and performance has not improved noticeably.

I have done some more testing and the performance bottleneck is in the selection.set function called here in the onClick handler. Specifically it is the message sent to Tree Style Tab in that function.

Example of message sent:

{
  state: Array [ "selected" ],
  tabs: Array(1091) [ 1, 3378, 3379, … ],
  type: "remove-tab-state"
}

Time before message was completed: 11 347 milliseconds.

Lej77 commented 5 years ago

Fortunately I see that this message can be skipped by holding the ctrl key when shift clicking and when I do that the operation completes quickly. So I can use that as a workaround.