Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TST-we is too slow to rebuild tree when reopen TST sidebar #1425

Closed
alice0775 opened this issue Oct 5, 2017 · 15 comments
Closed

TST-we is too slow to rebuild tree when reopen TST sidebar #1425

alice0775 opened this issue Oct 5, 2017 · 15 comments

Comments

@alice0775
Copy link

Short description

SSIA

Steps to reproduce

  1. Start Firefox with clean profile.
  2. Install TST.
  3. Open may tabs(more than 30tabs)
  4. Toggle TST sidebar

Expected result

The TST sidebar should appear as fast as bookmark sidebar

Actual result

It takes too much time(1-3sec)

Environment

  • Platform (OS): Windows10 CU 64nit
  • Version of Firefox: Nightly58.0a1 Build ID 20171005100211
  • Version (or revision) of Tree Style Tab: 2.0.2
@piroor
Copy link
Owner

piroor commented Oct 6, 2017

Current result of gMetricsData.toString() on my environment:

startup (reload):

0: Loaded
173: earlyInit start
0: earlyInit end
39: init start
2: configs.$loaded
109: applyStyle
179: waitUntilBackgroundIsReady and retrieveAllContextualIdentities
62: treestyletab:pull-tab-id-tables
103: rebuildAll
2: updateContextualIdentitiesStyle and updateContextualIdentitiesSelector
34: updateTabbarLayout
378: inheritTreeStructure
0: start to listen events
28: apply configs
273: treestyletab:request-registered-addons
53: treestyletab:request-scroll-lock-state
42: getting scroll-position
0: init end

reopen sidebar:

0: Loaded
111: earlyInit start
1: earlyInit end
14: init start
2: configs.$loaded
47: applyStyle
79: waitUntilBackgroundIsReady and retrieveAllContextualIdentities
5: treestyletab:pull-tab-id-tables
158: rebuildAll
2: updateContextualIdentitiesStyle and updateContextualIdentitiesSelector
28: updateTabbarLayout
378: inheritTreeStructure
1: start to listen events
34: apply configs
250: treestyletab:request-registered-addons
65: treestyletab:request-scroll-lock-state
17: getting scroll-position
0: init end

inheritTreeStructure() and treestyletab:request-registered-addons take time a lot.

@piroor
Copy link
Owner

piroor commented Oct 6, 2017

treestyletab:request-registered-addons simply does: browser.runtime.sendMessage() (in sidebar) => Promise.resolve(gExternalListenerAddons) (in background page) so I cannot believe that this takes much time. Hmm...

@seascape
Copy link

seascape commented Oct 6, 2017

Data point: I have a browser with over 500 tabs. TST takes around 50-60 seconds to finish loading, give or take.

This is with 5 major trees. The largest tree has ~170 tabs.

The loading time is much faster when I flatten the trees, about 26 seconds.

System is a 2010-era i7 x5650 @ 3.7 Ghz in Win10 Pro.

@piroor
Copy link
Owner

piroor commented Oct 7, 2017

By parallelized initialization, the startup time is reduced for me. gMetricsData.toString() on sidebar.html result on my environment:

total 2190 msec for 113 tabs
0: Loaded
34: earlyInit start
1: earlyInit end
5: init start
1: configs.$loaded
28: applyStyle, waitUntilBackgroundIsReady and retrieveAllContextualIdentities
0: applyUserStyleRules
5: calculateDefaultSizes
19: kCOMMAND_PULL_TAB_ID_TABLES
170: rebuildAll
52: updateTabbarLayout
16 (async): initializing contextual identities
13 (async): tabContextMenu.init
834 (async): getting kWINDOW_STATE_SCROLL_POSITION
1408: inheritTreeStructure
0: start to listen events
249: apply configs
1710 (async): main task: notify ,update, restore, and so on
1871 (async): getting registered addons and scroll lock state
1858 (async): parallel initialization tasks
217: applying scroll position
1: init end"

Before parallelization, it took 5000-6000msec for me.

@kkot
Copy link

kkot commented Nov 13, 2017

I have about 600 tabs an it takes 4 seconds to rebuild tree. It is not bad but of course faster is better especially after switching from History or Bookmarks back to Tree Style Tab.

@piroor
Copy link
Owner

piroor commented Nov 13, 2017

On 2.2.0, more optimization has landed.

@gagarine
Copy link

V 2.3.0 seem faster but still to slow ;). Also the loading is kind of strange: first the tabs appear, then the tree is made until the end their is a loader on top of the UI. This cause certainly lot of unnecessary repaint.

I didn't look at the code, but can't we keep a tab tree in memory and update it when tabs are opens?

Say that I don't understand why making this tree take so long. With 10 tabs it should take 0.001ms.

But we shouldn't draw the UI before data are ready I think.

@piroor
Copy link
Owner

piroor commented Nov 30, 2017

There are two cases of tree restorations.

  1. (relatively) quickly restored case
    • when you disable and enable TST
    • when you configure Firefox to restore last tabs and windows automatically on the startup
    • when you restart Firefox to apply updates
  2. very slowly restored case
    • when you choose "Restore Previous Session" from the hamburger menu
    • when you restore tabs after crash

The reason why case 1 is quick is: TST can assume that it is window restoration and can skip some operations. The restoted window have a session data stored by TST: the structure information of whole tabbar. On the other hand, there is no such hint to assume it is window restoration at case 2, so TST need to restore tree from relative relations of tabs step by step. WebExtensions doesn't have any event to notify Firefox is going to restore window at case 2. If we know that the restoration is started, we'll make the case 2 more quickly...

@SXZ1
Copy link

SXZ1 commented Nov 30, 2017

Stupid question: will this bug help?
https://bugzilla.mozilla.org/show_bug.cgi?id=1413525

@piroor
Copy link
Owner

piroor commented Dec 1, 2017

The bug 1413525 seems talking about APIs for addons like Session Manager. It won't help TST if no any new event is notified when a session is going to be restored, because TST won't include such session management system by self (it is quite out of the target range of this project).

@gagarine
Copy link

gagarine commented Dec 1, 2017

There are two cases of tree restorations

But this bug is not about restorations, it's about the tree stye pane slow to build when opened.

bkw2s2w8q7

@piroor
Copy link
Owner

piroor commented Dec 1, 2017

Oops, sorry, I misunderstood this to another issue about session restoration...

@piroor
Copy link
Owner

piroor commented Dec 1, 2017

https://github.com/piroor/treestyletab/tree/reopen-tabbar-from-cache experimental branch to try caching built DOM tree. Still there are many problems around information not stored as attributes - TST stores some information to HTML elements just as a JS property and it won't be serialized and need to be restored manually.

Or, possibly I need to rewrite everything based on virtual DOM, but it is very hard work for me and it will be the the version 3.0.

@piroor
Copy link
Owner

piroor commented Dec 2, 2017

Now the cache system is merged to master, but I think we need more doogfooding.

@piroor
Copy link
Owner

piroor commented May 1, 2019

I close this because outdated.

@piroor piroor closed this as completed May 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants