Bug 669034 (sessionRestoreJank)

[meta] Re-architect session restore to avoid periodic freezes

NEW
Unassigned

Status

()

--
major
8 years ago
8 months ago

People

(Reporter: jesup, Unassigned)

Tracking

(Depends on: 6 bugs, Blocks: 4 bugs, {arch, meta, perf})

Trunk
arch, meta, perf
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [snappy:p1] [tracking][fxperf], URL)

Attachments

(9 attachments, 1 obsolete attachment)

(Reporter)

Description

8 years ago
While investigating causes for periodic freezes in keyboard input (and mouse response, etc - see bug 667250) I found that one cause was the periodic saving of sessionstore.js.

The architectural problem is that saving the session is done in the foreground, blocking most or all other work by the browser (and often plugins).  I.e. if you have 500 tabs, many with full histories, and sessionstore is gathered and written to disk, it can freeze the browser for a couple of seconds even on a fairly fast quad-core Athlon 965.

This is a real problem for all sorts of reasons, and it can cause problems *long* before you get to 500 tabs.  It especially impacts anything that is in any way realtime or semi-realtime - typing, UI response, streaming, future work to support rtcweb, etc.  With the current goal of 50ms UI response time, this problem will be a major blocker to achieving that goal.

Saving the session should be a background task which blocks the foreground no longer than it takes to harvest a single tab's data (though I'd like it if it didn't even block on those if the tab is inactive).

Obviously this would be a major rewrite, and not simple either.

There are a number of filed bugs that partly or completely devolve to this issue; we should mark them as blocked by this or in some cases dup them to this.
(Reporter)

Comment 1

8 years ago
CC'd a bunch of core people to vet the architectural change suggested here (many grabbed from bug 490122 - my apologies if any aren't interested).  Read the full bug for info, but the overview is that sessionstore saving shouldn't block the main/UI thread.

I suspect implementing this would require pushing some parts of the session data collection and the file writing to a background thread and I'm guessing out of JS code and into C++, though I'm not familiar enough with what's possible nowadays to be 100% sure of that.  

It would need to collect the data in a safe way cross-thread (make requests to the foreground UI thread which are handled without fully tying up the foreground thread (retain interactivity during save), but also without the foreground thread being able to starve the sessionstore thread of the data it needs for "too long" (avoid failing to save or failing to save often enough).

This may already be under way for Electrolysis, or doing it may help with Electrolysis - I haven't looked into where that project stands or if this issue has come up.
Blocks: 490122, 667250
Keywords: arch, main-thread-io, perf
Bug 485976 moved file writing off the main thread. So data collection is going to be the only thing causing issues. I'm not sure we're doing everything as smart as we can be, so there are probably optimizations to be made first (and some have been made).

E10S might fix this but I don't know enough there to say.
(Reporter)

Comment 3

8 years ago
Yes, that matches the GDB captures -- all were in data collection.

As I commented in IRC:
We may be able to help (even in the current design) by caching info and avoiding regenerating it all when saving tabs that haven't changed.  There's some code to handle 'dirty' windows/tabs; I'm not sure how effective that is in reducing load.
What data is it collecting that takes long to collect?
Values in all form controls (together with construction of XPaths to identify the form controls) and scroll position (involves layout flushes) are the two obvious ones.
Does it do anything to avoid recapturing all the data for tabs that haven't changed since the last save?
(Reporter)

Comment 7

8 years ago
I'd need to spend more time figuring out the JS (my JS is rusty), but it appears we don't avoid it for the  normal case.  If we can do that *and* be fairly sure we don't miss state changes (make sure the dirty flag is accurate), then we could save a fair bit.  Certain cases would still have a moderate amount of overhead though.  Still, that could well be a stepping stone to a more complete solution, at the cost of extra work.  I doubt we can do this by tomorrow though.  I'll look tonight if I have time; at least try to get some targeted profiles.
That sounds like it would be a huge win, quite likely more than enough to declare this bug fixed.
(Reporter)

Comment 9

8 years ago
Huge win: yes, though maintaining the state may be a fair bit of work - that part may well carryover to any future improvements if it's accurate.  I'm worried that there will be too much overhead in fairly common cases to meet the stated goals of things like 50ms UI response time, however.  Doubly so on hardware that isn't powerful (mobile, etc - though those have a lot less tabs - netbooks might not, and people might sync a large list of tabs from a desktop).

I believe it's also saving session history, cookies (at least non-global ones), and perhaps some other stuff.  Let's see how much it is per active tab, and how much trouble it is to maintain an accurate dirty flag.  One should assume memory overhead would be on the order of the size of your sessionstore.js file, or some small integer multiple.  In my case that means circa 20-30MB (no about:sessionrestore open).  I'll do more investigation tonight, but I wouldn't bet on more than small improvements for tomorrow (I have other patches I want to get in, and anything major on this bug will probably require a lot more time and baking).

FYI I'm getting entire sentences ahead in my typing...
(Reporter)

Comment 10

8 years ago
If sessionrestore.js is moved to sqlite (allowing direct updates of tab's data), that might simplify things by making the logic per-tab instead of global, with some type of back-end thread for doing/writing the updates in batches.
(Reporter)

Comment 11

8 years ago
Created attachment 543871 [details]
JProf of saveState() in nsSessionstore.js

This is on a fresh load, 500 tabs, ~15 loaded, press and hold key in bugzilla comment input area.  Time to process saveState() was 1.5-2.0 seconds whenever it was invoked.  sessionstore.js was ~20MB.

js_json_stringify() is an obvious partial culprit - this probably processes the entire 20MB, and uses approaching 30% of the time.

However, that means that a lot is used elsewhere.  20% seems to be GC+CC, for example.  I'm running a second jprof excluding the stringify/write code to better focus on that.
(Reporter)

Comment 12

8 years ago
Created attachment 543872 [details]
JProf excluding saveStateObject() (i.e. js_json_stringify & write)

Ironically NewURI() shows up here at ~6%.  Obviously largely JS code, which is a little hard to really profile in JProf.  Lots of stuff with cookies.
(Reporter)

Comment 13

8 years ago
Looks like all the GC/CC is during stringify... so probably total 50% for that.
(Reporter)

Comment 14

8 years ago
Created attachment 543875 [details]
JProf excluding saveStateObject() (i.e. js_json_stringify & write) (updated)
Attachment #543872 - Attachment is obsolete: true
We've talked about moving it to a DB in the past, as one possible way to avoid writing out the _entire_ file whenever anything changes. Another idea was to append updates to sessionstore.js, and only regenerate the whole file much less frequently... Kind of a {a:1, b:2, c:3} + {b:99} = {a:1, b:99, c:3} thing.

It's a bit surprising that there's so much overhead in just collecting the data, though.

This would be a fascinating area for telemetry probes! For example, your 20MB/500tabs case seems to be an order of magnitude more data per-tab (on average, obviously), than my 160KB/35tabs. Fodder for a new bug, natch.
Hmm.  I'm at 2MB/100tabs (the latter is a ballpark figure, but I think pretty close).  I bet it really depends on what you have in your tabs...
First off, I'm going to say a sessionstore.js of 20-30mb is (almost certainly) an outlier. Even my session (200 tabs, 1.1mb) is probably outside of the normal. Doesn't mean we shouldn't fix things, but I wanted to put that out there.

I know we do some caching, but I think we discard things pretty aggressively. We could do something about that.

(In reply to comment #12)
> Ironically NewURI() shows up here at ~6%.  Obviously largely JS code, which
> is a little hard to really profile in JProf.  Lots of stuff with cookies.

The cookie stuff as a whole might be able to be cleaned up. I noticed (at least some of) the NewURI stuff the other day, and filed bug 668865 (on my list of sooner rather than later).
Paul, once again I'm seeing a bigger sessionstore file than you for more tabs...
Er, fewer tabs.
(Reporter)

Comment 20

8 years ago
Yes.  I'm trying to look (the 20MB line causes editors fits, though emacs can handle it, with some slowdown).

I've found multiple instances of "storage":{"http://www.google.com": entries followed by 200-800K (!) of encoded HTML (with lots of \\\).  There are some other sites that also cause large dumps. I'm trying to dig out exactly which tab it is...  it's a pain to read by hand.  :-)  I don't think it's a "normal" google page though.  (Books?  ACM digital library?)
(Reporter)

Comment 21

8 years ago
Created attachment 544160 [details]
snippet of sessionstore.js file with google HTML code

Here's a 280K item from a pretty-printed sessionstore.js file, with a single bit of data from google ("web-c" to "web-v") that takes up 242K of the 280K.  Not sure exactly where this is sourced from or why, but wow does it take up space.

To pretty-print JSON:  
Python 2.6 - "python -mjson.tool <sessionstore.js >session_pretty.js"
Python 2.5 - install simplejson.py (http://pypi.python.org/pypi/simplejson/)
   "python -msimplejson.tool <sessionstore.js >session_pretty.js"
(Reporter)

Comment 22

8 years ago
Created attachment 544161 [details]
Another snippet (even bigger - 618K)

And an even bigger one, at 618K.

While these google entries are by far the worst, windows referencing "babble.com" and "ancestry.com" tend to be sizable as well (just no where near this big).
Wouldn't that be a quick search page? (one where you type your query, and get the result as you type)
(Reporter)

Comment 24

8 years ago
Probably it is quick search.  Why they're so huge... dunno, but maybe there's a way to avoid saving such a huge amount of data.  In my dreams, we get them to change to stop causing this.

I saw some bits (found via google!) about web-c and "sessionStorage.removeitem()"....  worth following up tomorrow.  Maybe they try to avoid storing it on Chrome or IE and ignore us.
(Reporter)

Comment 25

8 years ago
Created attachment 544215 [details]
Clean sessionstore.js showing size issue

This sessionstore.js file was created by making a new profile, loading it, closing the first-time mozilla start page (leaving just about:home), typing google.com into the address bar, return, then typing "edward murphy".

That's it.  sessionstore.js is 366K.  Ouch.
(Reporter)

Updated

8 years ago
Depends on: 669603
(Reporter)

Comment 26

8 years ago
I spun the "Google make enormous sessionstore entries" off as bug 669603 (which now is a blocker for this bug), so this bug can remain focused on how to avoid pausing in any case (many things can bloat sessionstore.js)
(Reporter)

Comment 27

8 years ago
Created attachment 545115 [details]
Sysinternals Process Explorer graph of startup

This shows startup with the large profile (but only 15 tabs loaded).  Note the periodic writes of sessionstore.js.  Note also the huge memory bumps when it writes sessionstore.js (of around 100+MB).  This was with 5.0.

Looking at it in Aurora I see the same thing.  Also, once it's loaded and stable, when one of these sessionstore saves is done you still get the big bump - looks like ~120MB at the moment.  Sometimes it's released immediately, sometimes not (infamous fragmentation?  The grouping by compartments won't help here; it's probably in System Principal).  My guess is JSON-encoding it and writing it out uses (or uses and drops waiting for GC) 3-5 copies of the data temporarily.
(Reporter)

Comment 28

8 years ago
I posted a LONG discussion of the problem and a host of possible solutions in mozilla.dev.platform/mozilla.dev.performance.
URL for the post?
(Reporter)

Comment 30

8 years ago
Created attachment 546910 [details] [diff] [review]
Some testcode for jprof-ing sessionstore saves, and output the time they take
Whiteboard: [tsnap]
Target Milestone: --- → Firefox 8
Version: unspecified → Trunk
Target Milestone: Firefox 8 → ---

Updated

7 years ago
Blocks: 698500
Duplicate of this bug: 698514
Depends on: 698565
Alias: sessionRestoreJank
Duplicate of this bug: 624812
Depends on: 700923
The comments here show that a large profile is not required for Firefox to exhibit these problems, and that the freezes are not necessarily related to file IO, so I've updated the summary to reflect that.
Summary: Periodic freezing with larger profiles (architectural issue with saving sessionstore.js) → Periodic freezing while saving session
(Reporter)

Comment 35

7 years ago
I'm not sure I agree the comments here back up "The comments here show that a large profile is not required for Firefox to exhibit these problems".  I agree it's not IO; that is handled asynchronously.
"large profile" is not super descriptive, since the Firefox profile is a folder full of wildly unassociated files. But you filed this bug, so I'll use a different metabug for these issues if you'd like.

The thought behind my broadening of the summary was that in each of the dependent bugs even a single page of the possible 50 (default) in a single tab's session history could cause a freeze by either having a dumptruck full of sessionStorage data or a ton of subframe session history.
(In reply to Dietrich Ayala (:dietrich) from comment #36)
> The thought behind my broadening of the summary was that in each of the
> dependent bugs even a single page of the possible 50 (default) in a single
> tab's session history could cause a freeze by either having a dumptruck full
> of sessionStorage data or a ton of subframe session history.

Oops, correction - IIRC we'll only save the sessionStorage for the active page in the history. Problem still exists regardless.
(Reporter)

Comment 38

7 years ago
Perhaps I should have said "large sessionstore".  Thus far, I've seen no other slowups beyond those caused by a large sessionstore.js file.  I actually am ok with it morphing into a meta-bug, with dependencies to bugs that cause sessionstore to be large.

Oh, and while I'm sure it's now a failing battle in mozilla because someone decided it was cute and started using it, I *hate* the use of "jank" here.  So far as I can tell, someone re-purposed "jank" around the beginning of the year to mean UI slowups; the normal slang definition has been around a lot longer and is purposely non-specific.  In fact, the first usage I can find on the web was someone modifying a slang dictionary to add this definition, with no comments.
(In reply to Randell Jesup [:jesup] from comment #38)
> "jank"

IIRC the Chrome team used it to describe their UI responsiveness work, so we are copying their usage of it. ___________________. (<- obvious copying-Chrome snark)

I do not care what we call it, as long as we fix the problems. You are welcome to use whatever terms you like, as long as they meet the Bugzilla etiquette guidelines ;)
(Reporter)

Updated

7 years ago
Whiteboard: [tsnap] → [tsnap][snappy]
Whiteboard: [tsnap][snappy] → [snappy]

Updated

7 years ago
Whiteboard: [snappy] → [snappy:p1]
[W.r.t. whiteboard annotations:  in MemShrink, we mostly avoid assigning priorities until the MemShrink meeting.  Even if the priority is obvious, that gives everyone who attends the meeting the chance to see the bug and CC themselves if they aren't already CC'd.]
[oh, it's entirely possible that you added the priority in the Snappy meeting.  If so, please ignore me!]
In bug 705597 I describe how I have a 6MB sessionstore.js file that I reduce down to 880k simply by removing 'about:blank' entries. Is that related?
Depends on: 705597
Assigning to Dietrich. This bug might need to be re-prioritized/reassigned.
Assignee: nobody → dietrich
I just had a closer look at my ~670KB sessionstore.js file, containing ~135 tabs in one window.

The one thing that clearly stood out were the seven app tabs I have. In the pretty printed version of sessionstore.js (~1.05MB), those alone took ~4350 of the total ~14400 lines.

Of these seven app tabs, the worst by far is Google Reader, taking nearly 1500 lines alone. All the others, including two Gmail tabs, took about 500 lines, each.

Concentrating on the Google Reader tab, except for not even 10 other lines, it's entire size is caused by the 50 entries of history. None of these entries had any about:blank subframes in them, they all looked fine to me: One document with three subframes. The only thing that might be slightly interesting is that each of these subframes had the exact same URL and referrer in all of the 50 history entries, but as that's probably just an artifact of the way Reader is implemented, it doesn't look that interesting after all.

Comparing the Reader tab with the others again, I confirmed that the only difference is that all other apps don't have any subframes stored in their history entries.

-----------

Based on this analysis, maybe it should be considered to reduce the amount of history entries stored for app tabs? I can't imagine any app workflow that would involve going back more than, say, 10 entries in an app. Spanning sessions, maybe even 5 would suffice. That would mean reducing the size of app tabs in sessionstore.js by nearly 90%. Supposing that the pretty-printed version of my history is representative of the non-pretty-printed one (which seems reasonable to me, given that all entries have a very similar structure), that alone should shave off about 180KB or more than 25% from my sessionstore.js.
(Reporter)

Comment 45

7 years ago
Note: relabeled this bug to keep it focused on one issue - rearchitecting sessionstore harvesting and saving to remove or minimize the amount of time the main thread is blocked.  Handling sessionstore on a background thread, caching data, variant of the e10s session saving work, etc are all options.
Keywords: main-thread-io
Summary: Periodic freezing while saving session → Re-architect sessionstore.js saving to avoid periodic freezes
(Reporter)

Comment 46

7 years ago
Comment hoisted from bug 669603 comment 31 by Deitrich:

* i see lots of synchronous function calls that could do lots of work and end up blocking the UI: History.getDomains(), session history stuff, domStorage reading and writing. what do you think about using this opportunity to think about using some common async patterns - such as function(options, callback(error, result)) like a lot of node.js code uses, or some variation?
I feel I have to give another real world example.
After my FF 10beta was almost unusable with its UI responsiveness I figured it was caused by sessionstore.js which was 7MB for just 4 app tabs and 1-3 normal tabs. The app tab contains gmail which seems to be one reason for a lot of data. Unfortunately I deleted the file already.
Apart from architectural issues with sessionstore.js I wonder if it's needed or expected to save away 7MB of session data.
Has something regressed from FF9?

Wolfgang, have you seen similar problems before FF10?
tracking-firefox10: --- → ?
I remember I removed sessionstore.js once before (probably FF8) but it was worst performance experience ever with FF10.
Tracking for FF10 as it pertains to the investigation for bug 711900. catlee, bz (who actually shared his session restore with us), and dmandelin all though that session restores may have been contributing to their post-FF10 issues. It'd be good to get in touch with them as a possible next step.
tracking-firefox10: ? → +
This code has been this way for years, and this is waaaay past the safety thresholds for beta (even assuming we could get this done in time).
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #51)
> This code has been this way for years, and this is waaaay past the safety
> thresholds for beta (even assuming we could get this done in time).

Yeah, Dietrich has since let me know that this is almost certainly unrelated to 711900. We can untrack.
tracking-firefox10: + → -
Depends on: 708488
Depends on: 697903
See also bug 516755, the conversion work for e10s.
Keywords: meta
Depends on: 681201
Depends on: 711193
Depends on: 715612
Depends on: 532150
Depends on: 535519
Depends on: 394492
I understand that the sychronous behaviour to collect, create and write out the json based sessionstore is probably be changed in a dependent bug.

From the other side I'm wondering if all data collected in sessionstore is needed for real.
I've just scanned my (again about 5MiB) sessionstore JSON string and found the biggest part (around 75%) is being used by entries from twitter.com (https://api.twitter.com/receiver.html actually). There is also a good part of Facebook entries (probably 20%), I have to add that I basically monitor these pages but do only send ocassionally.
Thanks Wolfgang. Can you provide more detail about what exactly the entries are comprised of? Eg: Are they tab history entries? or frames? or form data?
(In reply to Wolfgang Rosenauer [:wolfiR] from comment #54)

Is your profile brand new with a latest build?  Or an old profile?  I'm curious, cause I though something like this was addressed already.
(In reply to Dennis "Dale" Y. [:cuz84d] from comment #56)
> Is your profile brand new with a latest build?  Or an old profile?  I'm
> curious, cause I though something like this was addressed already.

I don't wipe my profile regularly so it's possibly quite old. What part of the profile would hold a fix back from working?
What is a latest build? I run 11.0b2 currently.

(In reply to Dietrich Ayala (:dietrich) from comment #55)
> Thanks Wolfgang. Can you provide more detail about what exactly the entries
> are comprised of? Eg: Are they tab history entries? or frames? or form data?

I don't know the data structure of sessionstore so I can only paste some example entries (see below). That type of sets is repeating around 4000 times.

                                    {
                                        "ID": 1997552426, 
                                        "docIdentifier": 239, 
                                        "docshellID": 20, 
                                        "owner_b64": "NhAra3tiRRqhyKDUVsktxQAAAAAAAAAAwAAAAAAAAEYAAQAAAAAAAd6UctCANBHTk5kAEEug/UA5X+BFfRhK26P9r5jIoa8RAAAAAv////8AAAG7AQAAABVodHRwczovL3R3aXR0ZXIuY29tLyMAAAAAAAAABQAAAAgAAAALAAAACP////8AAAAI/////wAAAAgAAAALAAAAEwAAAAIAAAATAAAAAQAAABMAAAABAAAAFAAAAAAAAAAU/////wAAAAD/////AAAAE/////8AAAAVAAAAAAEAAAAAAAAAAAABAd6UctCANBHTk5kAEEug/UA5X+BFfRhK26P9r5jIoa8RAAAAAv////8AAAG7AQAAABVodHRwczovL3R3aXR0ZXIuY29tLyMAAAAAAAAABQAAAAgAAAALAAAACP////8AAAAI/////wAAAAgAAAALAAAAEwAAAAIAAAATAAAAAQAAABMAAAABAAAAFAAAAAAAAAAU/////wAAAAD/////AAAAE/////8AAAAVAAAAAAEAAAAAAAAAAAABAA==", 
                                        "subframe": true, 
                                        "url": "about:blank"
                                    }, 
                                    {
                                        "ID": 1997552427, 
                                        "docIdentifier": 240, 
                                        "docshellID": 19, 
                                        "referrer": "https://twitter.com/", 
                                        "subframe": true, 
                                        "url": "https://api.twitter.com/receiver.html"
                                    }, 
                                    {
                                        "ID": 1997552428, 
                                        "docIdentifier": 241, 
                                        "docshellID": 92, 
                                        "referrer": "https://twitter.com/", 
                                        "subframe": true, 
                                        "url": "https://api.twitter.com/receiver.html"
                                    },

Updated

7 years ago
Duplicate of this bug: 670765
Depends on: 716174
Depends on: 726235
Depends on: 745040
 I don't believe Dietrich is planning to work on this anymore
Assignee: dietrich → nobody
...but maybe ttaubert is?
(In reply to Justin Dolske [:Dolske] from comment #60)
> ...but maybe ttaubert is?

Indeed, the current plan is to fix and land these bugs in order:

bug 745040 -> bug 708488 -> bug 742047 -> bug 669603

We then have good base to continue working on bug 669603 which will make sessionStore (de)serialization as async as possible.

Comment 62

7 years ago
(In reply to Paul O'Shannessy [:zpao] (no longer moco, slower to respond) from comment #17)
> First off, I'm going to say a sessionstore.js of 20-30mb is (almost
> certainly) an outlier. Even my session (200 tabs, 1.1mb) is probably outside
> of the normal. Doesn't mean we shouldn't fix things, but I wanted to put
> that out there.
> 
> I know we do some caching, but I think we discard things pretty
> aggressively. We could do something about that.
> 
> (In reply to comment #12)
> > Ironically NewURI() shows up here at ~6%.  Obviously largely JS code, which
> > is a little hard to really profile in JProf.  Lots of stuff with cookies.
> 
> The cookie stuff as a whole might be able to be cleaned up. I noticed (at
> least some of) the NewURI stuff the other day, and filed bug 668865 (on my
> list of sooner rather than later).

My sessionstore.js file is always above 20mb and right now is above 50mb. It crashes the browser often and sometimes stops updating itself for 20min before it crashes.
It would be very nice to have some code that could be pasted into js console and dumps out anonymized statistics about the sessionstore.js file contents.

I'm betting it's bug 669603, but would good to be able to have reporters easily confirm that.
(Reporter)

Comment 64

7 years ago
I'd bet strongly it's that "bug"/design feature.  Mine sits at 20MB currently.  Any tab hoarder has this problem, and even non-tab-hoarders who always start out at Google.

Comment 65

6 years ago
I was looking the new profiler improvements and the "show jank only" while I saw that :
http://people.mozilla.com/~bgirard/cleopatra/?report=cdbb5f3fffe2f824c10d28718c9d7bec2571e16c

60% of the time is allocated to ssi_serializeHistoryEntry()!

Hope this will help devs.

PS : 25 tabs were opened.
(In reply to Jean Claveau from comment #65)
> I was looking the new profiler improvements and the "show jank only" while I
> saw that :
> http://people.mozilla.com/~bgirard/cleopatra/
> ?report=cdbb5f3fffe2f824c10d28718c9d7bec2571e16c
> 
> 60% of the time is allocated to ssi_serializeHistoryEntry()!
> 
> Hope this will help devs.
> 
> PS : 25 tabs were opened.

It's handy. It would be even more handy if you used windows, so you could capture the C++ stack too :)

Based on this profile, I'm guessing that breaking up the for loop in saveWindowHistory http://dxr.mozilla.org/mozilla-central/browser/components/sessionstore/src/SessionStore.jsm.html#l1863 into multiple callbacks will improve responsiveness significantly. Tim, what do you think? This would involve adding some logic to make sure code copes with tabs mutating it under it. Might be worth the hassle.
(Reporter)

Comment 67

6 years ago
Oh yeah, that loop needs to be interruptible.  :-)  25 tabs is nothing; I typically have 200-900 tabs, with 50-150 loaded.  There is a significant minority of users who are "tab hoarders", especially now that we don't reload all tabs by default on startup.
Fwiw, since bug 794091, output is now off main thread.

Also, FX_SESSION_RESTORE_COLLECT_DATA_MS seems to suggest that less than 3% of users have a collection that lasts longer 50ms+ and less than 0.3% a collection that lasts 180ms+ (still trying to figure out how to interpret history).
Summary: Re-architect sessionstore.js saving to avoid periodic freezes → [meta] Re-architect the session store to avoid periodic freezes
Summary: [meta] Re-architect the session store to avoid periodic freezes → [meta] Re-architect session restore to avoid periodic freezes
Duplicate of this bug: 774163
Depends on: 912717

Comment 70

5 years ago
So, Firefox 24.0, I only have a reasonable number of tabs, and used to see stalls of 3000 ms (yes, 3 seconds) in the parent of ssi_serializeHistoryEntry(), as well as 800 ms to write the sessionstore.js file to disk. I have a quad-core Haswell Xeon and a fast Intel SSD. The sessionstore.js file was 60MB.

I tracked it down to some ajax-y query mechanism Facebook chat uses that leaks long (>1kB) urls which all get persisted.

From a little python script I wrote to sort strings in the JSON by length, outputting keys:
1212 /windows[0]/tabs[5]/entries[5]/children[14]/url
1212 /windows[0]/tabs[5]/entries[5]/children[4]/url
1212 /windows[0]/tabs[5]/entries[5]/children[8]/url

These URLs are of the form "https://www.facebook.com/ai.php?something=1kB_of_gobblydegook". The result of GETing this 1kB URL is a 0.10kB response:

<span id="fbEmuTrackingSucc&#101;ss">Succ&#101;ss</span>

Those leaking URLs accumulated 10-50kB per minute towards my sessionstore.js, eventually leading to 3000 ms stalls every 10 seconds, which is about as fun as it sounds. I used adblock to block the URLs, and now my sessionstore.js is fairly stable at around 100-120 kB.

I'm not sure what can be done from the Firefox side to garbage collect this sort of behavior (it's hard to destroy manually without losing your session because of tab-undo); it could certainly be fixed on Facebook's end, but their public bug-filing mechanism is pretty bad.

Let me know if I can provide more information. Thanks.
> I tracked it down to some ajax-y query mechanism Facebook chat uses that
> leaks long (>1kB) urls which all get persisted.

I wonder if bug 934391 is related.

Comment 72

5 years ago
Wrote an extension that may be useful in hunting down the offending sites in issues like the one described above, and in flushing the tab-undo list:
https://addons.mozilla.org/en-US/firefox/addon/about-sessionstore/
Thanks, C, davidtryse. Filed this as bug 934934.

Comment 74

5 years ago
(In reply to Nicholas Nethercote [:njn] from comment #71)
> > I tracked it down to some ajax-y query mechanism Facebook chat uses that
> > leaks long (>1kB) urls which all get persisted.
> 
> I wonder if bug 934391 is related.

I did notice my memory usage go down from 1600 MB to about 1100 MB in about:memory.

Comment 75

5 years ago
(In reply to Nicholas Nethercote [:njn] from comment #71)
> > I tracked it down to some ajax-y query mechanism Facebook chat uses that
> > leaks long (>1kB) urls which all get persisted.
> 
> I wonder if bug 934391 is related.

Also, yes, those symptoms in comment 8 sound very familiar to me.

(In reply to davidtryse+bz from comment #72)
> Wrote an extension that may be useful in hunting down the offending sites in
> issues like the one described above, and in flushing the tab-undo list:
> https://addons.mozilla.org/en-US/firefox/addon/about-sessionstore/

Thanks!

(In reply to David Rajchenbach Teller [:Yoric] <needinfo? me> from comment #73)
> Thanks, C, davidtryse. Filed this as bug 934934.

Great, thanks David.

Comment 76

5 years ago
There needs to be a way to clean up an existing sessionstore file from repetitive throw-away information such as that Facebook URL and closed tabs with no data or only containing repetitive data.

And then a method to fix/recover a very large and corrupt sessionstore file which currently does not allow restoring a session, and then perhaps build a new sessionstore file from it.

davidtryse+bz's extension goes a long way into investigating and cleaning up the sessionstore file's closed tabs and windows in bulk when the browser is in session with that file; the extension does not facilitate picking and choosing what to keep and what to delete.
Please have an look on my bug:
Bug 937651 - Replace the sessionstore.js with an sessionstore.sqlite
Depends on: 937651
Depends on: 942601
In Bug 937651, on Comment 12, David Rajchenbach Teller [:Yoric], wrote:
'I suspect that for most users, it is less than 300kb.'

If I open an online editor - no matter if on wikipedia or facebook - ss.json grow very fast.

I think this is an design problem, so I post it here.

I upload an screenshot of ss.json with tabs from wikipedia and facebook ...
Created attachment 8338621 [details]
sessionstore with tabs of online editors from wikipedia and facebook

Comment 80

5 years ago
Comment on attachment 8338621 [details]
sessionstore with tabs of online editors from wikipedia and facebook

Why shouldn't Firefox throw away form data (only) for tabs immediately once they are closed?  This would alleviate the problem, and could also be combined with throwing away form data once pages are more than one away from the current page in the history. Saving form data for the current page, plus 1 backwards and 1 forwards (if there is that history) is useful, but one doesn't need as much history for form data as for URLs (and Titles).

Comment 81

5 years ago
Firefox should make "browser.sessionstore.max_tab_history_backwards_forwards" settings in "about:config" with 0 being set as default behaviour, and other figure as the limit.

The issue is that even if I am not power user and I do not need deep backwards/forwards history within a tab, I can steel get bad experience. For example, one of my Google search tabs right now occupies more than 1.1 MB of space, according to "about:sessionstore". It has 11 backwards cashes states, the first of which is blank "google.co.uk" page. I look at it, and it turns out that Google's site stores tremendous amount of verbose styling along with the pictures of videos (if there are any in search results) within SessionStore, rather than separately in outer cache.

So those settings might need adjusting anyway.
(In reply to Seb A from comment #80)
> Why shouldn't Firefox throw away form data (only) for tabs immediately once
> they are closed?  This would alleviate the problem, and could also be
> combined with throwing away form data once pages are more than one away from
> the current page in the history. Saving form data for the current page, plus
> 1 backwards and 1 forwards (if there is that history) is useful, but one
> doesn't need as much history for form data as for URLs (and Titles).

Could you file a bug with these suggestions?

(In reply to Tobias B. Besemer from comment #78)
> In Bug 937651, on Comment 12, David Rajchenbach Teller [:Yoric], wrote:
> 'I suspect that for most users, it is less than 300kb.'
> 
> If I open an online editor - no matter if on wikipedia or facebook - ss.json
> grow very fast.
> 
> I think this is an design problem, so I post it here.
> 
> I upload an screenshot of ss.json with tabs from wikipedia and facebook ...

Let's not speculate. We'll know more once bug 942063 returns results, and even more once bug 942340 has landed.

(In reply to User Dderss from comment #81)
> Firefox should make
> "browser.sessionstore.max_tab_history_backwards_forwards" settings in
> "about:config" with 0 being set as default behaviour, and other figure as
> the limit.
> 
> The issue is that even if I am not power user and I do not need deep
> backwards/forwards history within a tab, I can steel get bad experience. For
> example, one of my Google search tabs right now occupies more than 1.1 MB of
> space, according to "about:sessionstore". It has 11 backwards cashes states,
> the first of which is blank "google.co.uk" page. I look at it, and it turns
> out that Google's site stores tremendous amount of verbose styling along
> with the pictures of videos (if there are any in search results) within
> SessionStore, rather than separately in outer cache.

I believe that this is tracked in bug 669603.

Comment 83

5 years ago
Created attachment 8338920 [details]
Profile screenshot for save session data.

Takes 5s on Haswell Xeon w/ SSD.  sessionstore.js is only 188 kB. Why!? :(((

Comment 84

5 years ago
(In reply to David Rajchenbach Teller [:Yoric] <needinfo? me> from comment #82)
> (In reply to Seb A from comment #80)
> > Why shouldn't Firefox throw away form data (only) for tabs immediately once
> > they are closed?  This would alleviate the problem, and could also be
> > combined with throwing away form data once pages are more than one away from
> > the current page in the history. Saving form data for the current page, plus
> > 1 backwards and 1 forwards (if there is that history) is useful, but one
> > doesn't need as much history for form data as for URLs (and Titles).
> 
> Could you file a bug with these suggestions?

Filed as Bug 943830 - (sessionRestoreJank) Firefox could throw away form (and storage?) data for tabs earlier than URLs and Titles

Should I add a dependency on it?  I'll leave that up to someone else.
No longer blocks: 981530
Whiteboard: [snappy:p1] → [snappy:p1] p=0
No longer blocks: 950073
Whiteboard: [snappy:p1] p=0 → [snappy:p1] [tracking]
A discussion about changes/improvements to the Session Restore (sessionstore):
https://groups.google.com/forum/#!topic/mozilla.dev.platform/JHrOP3yMgfg
Seems the "sessionstore.js"-file is gone ...
Would be nice to remove the backup-files now, too.
I have two ones on Win7 ("sessionstore.bak" and "sessionstore.bak-20140721004001") in C:\Users\[User-Name]\AppData\Roaming\Mozilla\Firefox\Profiles\[Profile-Name] and some old files in the subfolder "sessionstore-backups" called "previous.js" and with the naming-style "upgrade.js-20140830004002".
Fill a new bug for it ???

Also it seems https://wiki.mozilla.org/Firefox/session_restore don't cover the new changes ...
Where is the place (URL) to discus the documentation ???
tracking-firefox10: - → ---

Updated

3 years ago
Blocks: 809432

Updated

3 years ago
Duplicate of this bug: 809432
No longer depends on: 810932
Blocks: 1330635
Depends on: 1356605
Whiteboard: [snappy:p1] [tracking] → [snappy:p1] [tracking][fxperf]
You need to log in before you can comment on or make changes to this bug.