can take 4x longer to stringify for large sessions. https://bugzilla.mozilla.org/show_bug.cgi?id=698514#c1 need to figure out: * which keys need to be removed manually * if there are keys we can hang off tabs/windows/etc instead of the data objects * if there are keys we can safely live with unnecessarily writing and restoring

keys: _tabStillLoading _hosts _formDataSaved _shouldRestore _host _scheme and sometimes __lastSessionWindowID

Further testing shows this makes a massive difference. In a test session of 3.9mb worth of session data (due to a totally f'd up Facebook page), I was seeing hangs for 5-10 seconds. Disabling the key exclusion made the hangs all but disappear.

Ok, I'll make this a priority. I think we're probably ok to just keep those keys & associated data in the string we output. I'll need to check, but we ignore most, if not all, of those when reading data already. For those that we don't ignore, we can start ignoring them. (In reply to Dietrich Ayala (:dietrich) from comment #1) > _tabStillLoading OK - We set this on restore for each tab to short-circuit data collection, but it gets wiped once we save state properly - could hang on tab/browser. > _hosts OK - We never read this - it gets reset fairly often - could hang on window. > _formDataSaved OK - Only used to short-circuit, never read - could hang on tab/browser. > _shouldRestore OK - Never read - can't hang it on a window (it's used for closed window data) > _host OK - Not explicitly read, but if we don't finish restoring, might be used (but in that case it would be accurate) - with some work, it could hang on tab/browser (it's 1 per shistory entry, but could push an array up) > _scheme OK - same as _host > and sometimes __lastSessionWindowID OK (I think) - it's only used during deferred session restore (but would get reset then anyway). Even if it's not reset, I think it's ok. - can't hang on a window (used for association with unrestored data)

I was overly optimistic. This shows some improvement, but in my pathologically-big sessionstore.js, there are still sessionstore-related hangs due to other issues. Absolutely worth doing, just not as complete a culprit as I'd thought.

Patches coming (a performance in 6 parts), but here's the breakdown... (In reply to Paul O'Shannessy [:zpao] from comment #3) > > _tabStillLoading > OK - We set this on restore for each tab to short-circuit data collection, > but it gets wiped once we save state properly - could hang on tab/browser. Hung this on the tab. > > _hosts > OK - We never read this - it gets reset fairly often - could hang on window. Stored in a separate object now > > _formDataSaved > OK - Only used to short-circuit, never read - could hang on tab/browser. Hung this on the browser > > _shouldRestore > OK - Never read - can't hang it on a window (it's used for closed window > data) This will now be filtered out when quitting (as soon as it's used in the quitting process, it will get deleted so it won't end up on disk after a clean quit) > > _host > OK - Not explicitly read, but if we don't finish restoring, might be used > (but in that case it would be accurate) - with some work, it could hang on > tab/browser (it's 1 per shistory entry, but could push an array up) > > > _scheme > OK - same as _host Ohai bug 690992. I changed this up a bit & I'm hanging the data on the browser now. > > and sometimes __lastSessionWindowID > > OK (I think) - it's only used during deferred session restore (but would get > reset then anyway). Even if it's not reset, I think it's ok. - can't hang on > a window (used for association with unrestored data) It's ok to leave this in since the values on disk will be overwritten in the case where it's relevant. During a restore-at-startup process, these will get deleted to prevent unnecessary perpetuation on disk.

1/6: Stop excluding keys.

2/6: Move _tabStillLoading from data to the browser

3/6: Move _formDataSaved from data to the browser

4/6: Move _hosts from data to a new internal data object I didn't attach to each window because I didn't want to constantly be looking up a window by it's _SSi. That would have required more in-depth changes that aren't really necessary.

5/6: Move _hosts & _scheme out of each entry and onto the browser. This also involved a bit of rethinking how we do this. Now instead of having to look at each entry (often recursively), we'll push all the host data into a single array on each browser. During the session we only have to iterate over this flat structure.

6/6: Clear out lastSessionWindowID and shouldRestore when possible.

https://hg.mozilla.org/integration/fx-team/rev/8a62b79f6382 https://hg.mozilla.org/integration/fx-team/rev/e46a0677db7c https://hg.mozilla.org/integration/fx-team/rev/0606e23bc306 https://hg.mozilla.org/integration/fx-team/rev/74df455a5125 https://hg.mozilla.org/integration/fx-team/rev/d57acad5beb4 https://hg.mozilla.org/integration/fx-team/rev/3b2109405ac7

https://hg.mozilla.org/mozilla-central/rev/8a62b79f6382 https://hg.mozilla.org/mozilla-central/rev/e46a0677db7c https://hg.mozilla.org/mozilla-central/rev/0606e23bc306 https://hg.mozilla.org/mozilla-central/rev/74df455a5125 https://hg.mozilla.org/mozilla-central/rev/d57acad5beb4 https://hg.mozilla.org/mozilla-central/rev/3b2109405ac7

Just for my clarification, this makes session restore files backwards-incompatible, right?

(In reply to neil@parkwaycc.co.uk from comment #14) > Just for my clarification, this makes session restore files > backwards-incompatible, right? Incorrect. These things haven't been stored on disk for a while and for the most part still aren't. Even for the 2 things that might end up on disk now (_shouldRestore and _lastSessionWindowID) they aren't used in the startup process. So new files should load up just fine in Firefox 4 and visa versa.

Comment on attachment 572621 [details] [diff] [review] Patch 2 v0.1 Review of attachment 572621 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/sessionstore/src/nsSessionStore.js @@ +1701,5 @@ > > if (!browser || !browser.currentURI) > // can happen when calling this function right after .addTab() > return tabData; > + else if (browser.__SS_data && browser.__SS_tabStillLoading) { No need to test browser.__SS_data now.