[Bug] Closing multiple tabs is slow if animation is disabled

[valadaptive]

Abstract

When closing multiple tabs, it appears that updateVisualMaxTreeLevel is called once per tab closed if animation is disabled.

Every time a tab is closed, reserveToUpdateVisualMaxTreeLevel is called, which is supposed to queue/debounce calls to updateVisualMaxTreeLevel.

If animation is enabled, reserveToUpdateVisualMaxTreeLevel sets a timeout to ensure that updateVisualMaxTreeLevel is only called when the tabs are all finished closing. However, if animation is disabled, reserveToUpdateVisualMaxTreeLevel will always call updateVisualMaxTreeLevel.

This is a problem because not only does updateVisualMaxTreeLevel require an expensive CSS update, but it looks through every tab in order to determine the maximum tree level. This makes performance quadratic.

For example, let's say you have 200 tabs open. That's a lot, so you decide to clean up after yourself and close 150 of them. Here's what will happen:

• Tab 1 is removed. TST calls reserveToUpdateVisualMaxTreeLevel, and it queries 199 tabs to figure out what the max tree level is.
• Tab 2 is removed. reserveToUpdateVisualMaxTreeLevel queries 198 tabs.
• Tab 3 is removed. reserveToUpdateVisualMaxTreeLevel queries 197 tabs.
• Tab 4 is removed. reserveToUpdateVisualMaxTreeLevel queries 196 tabs.
• [...]
• Tab 150 is removed. reserveToUpdateVisualMaxTreeLevel queries 50 tabs.

In total, we end up making 150 individual CSS updates and performing queries on 18825 tabs. This gets much worse the more tabs you close--if you close 1000 tabs, for instance, TST will search through at least 500,500 tabs.

Steps to reproduce

1. Start Firefox with a clean profile and open a bunch of tabs (preferably at least 100).
2. Install TST.
3. Enable Firefox Profiler.
4. Uncheck "Enable animation effects" in the settings panel.
5. Open the profiler dropdown menu (it's in the addon tray in the top right), select the "Firefox" preset, and start profiling.
6. Right-click the bottommost tab in the sidebar and click "Close Multiple Tabs" -> "Close Tabs to the Top".
7. Wait for TST to become responsive again, then click the profiler icon to stop profiling.
8. Observe the resulting performance profile, and note how much time is spent in reserveToUpdateVisualMaxTreeLevel.

Here's a performance profile that I recorded which demonstrates the problem.

Expected result

Closing multiple tabs should be relatively fast.

Actual result

Closing multiple tabs freezes the entire WebExtensions process for a good few minutes, and it gets worse the more tabs you have open.

Environment

• Platform (OS): Linux
• Version of Firefox: 113.0.1
• Version (or revision) of Tree Style Tab: 3.9.17

[irvinm]

@piroor, I just wanted to make sure you saw this one. Still has the "needs-triage" tag.

[piroor]

Thank you for detailed investigation! I agree that the current mechanism can trigger too many function calls to update tree indentation. Such needless calls needs to be reduced.

My concern about the PR #3384 is: it will introduce a regression around some situations like tree collapsion/expansion. With the present version, you'll see shallowly indented tree without delay when you expand a deeply nested tree. But with the change you proposed, you'll see "very deeply indented tree" at first and it will be updated to "shallowly indented tree" with a delay. (TST automatically adjusts indent width of tree based on currently visible tree at all time.) I'm afraid that such a visual flicking may stress people suffered from an epileptic seizure - we need to remind that the disabled animation mode is used by no only performance oriented people but handicapped people also.

Anyway the performance issue you mentioned is really serious with large number tabs. As a compromise, I've introduced a change. The main idea is based on your PR but there is a difference: the delayed update is activated only when the function is called over 10 times at a time. I hope the change helps you.

[piroor]

After more experiments, now TST updates max indent of collapsed/expanded tree with less function call and without delay. Of course the threshold mechanism is also still available and it should be effective for other cases like reported here - it is mostly equivalent to your PR. Thanks to mention about this performance issue I missed!