711465 - Reconsider DOM session storage behavior

Status: NEW

(Firefox :: Session Restore, defect)

Description

[Randell Jesup [:jesup] (needinfo me)]

DOM Session storage is retained across crashes, which is specifically allowed by the spec.  We also appear to restore session storage data across a quit and restore-last-session, which is mildly contrary to the intent of the spec, though not disallowed that I know of.

IE8 doesn't restore the data across crashes, and I assume it doesn't restore it across a quit-and-restart.  I assume IE9 does save it across crashes, but that's a shaky assumption.  Chrome apparently doesn't restore it under any circumstance.

I don't know if session storage data is cleared on a history clear (it probably should be).

Various patches are under consideration that may affect these behaviors, such as in bug 669603 (to store the data in the cache, which would cause it to be lost on a crash).

Our options are:
a) Current behavior - restore session storage data on restore-last-session after a quit or a crash
b) Restore session storage data on restore-last-session after a crash only (more in keeping with the spec)
c) Never restore session storage data (like Chrome and maybe IE8)
d) Restore session storage data on restore-last-session after a quit, but NOT after a crash (current patch in bug 669603).

Note: in crash cases, there's always a chance the data won't be recoverable.

Explicitly choosing a behavior will set what our options for improving the performance are.

Comment 1

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

For session restore we've always pushed the boundaries of "session" specs. Some people don't like that we restore session cookies (see bug 443354, bug 530594), but the definition of "session" isn't strictly defined.

> mildly contrary to the intent of the spec, though not disallowed that I know of.
describes it pretty well.

Personally, I really like that behavior and it's something that (I think) has become expected of Firefox's session restore (even if we can't always get along).

So my 2 cents is that we should keep current behavior as best as possible.

Comment 2

[(dormant account)]

I think Randell's motivation here is pauses during session-store saving caused by this. Do we know how much this feature is costing us?

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

Correct (bug 669034, etc). The proposal/patch in bug 669603 helps solve the pauses (I believe), but would cause a significant regression in user-visible-behavior (loss of data on a crash).

There are other ways to deal with this, some of which were (unfortunately) discussed in m.d.performance (which due to the decision to delete it, the archives of which have disappeared, though I understand they're not lost).  The group is probably still archived elsewhere on the web (google groups perhaps).

LevelDB, flat files (a fair bit of work), storing the session history as some form of journal effectively to avoid full re-writes (combined with dirty bits), etc, etc.

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

See http://old.nabble.com/SessionStore-size---Bloat-in-sessionstore.js-from-Google-and-others-causing-major-UI-freezes-tt32091916.html#a32091916 for the original discussion from mozilla.dev.performance

Comment 5

[Asa Dotzler [:asa]]

What is the actual cost to us on shut down and start up? Is this only problematic with large uses? What sites use this for what purposes?

Comment 6

[(dormant account)]

We should revisit this once Dietrich & Co are done with bug 669034

Comment 7

[Denoyse]

What is the default lifetime for a cookie based browser session? Shouldn't session storage be deleted after a certain timeout?

Also: Why is session storage not deleted through if active logins are deleted? This way, session storage survives a firefox restart deleting all private data, while local storage doesn't...

And as cookie sessions seem to be restored on a firefox crash, session storage should survive a crash too.

Comment 8

[Tobias B. Besemer [:BesTo] (QA)]

e) Restore session storage data on restore-last-session after a quit, but NOT closed tabs & windows. ;-)

Comment 9

[Tobias B. Besemer [:BesTo] (QA)]

Attachment: Waste of sessionstore through keep data from closed tabs & windows

Screenshot of the output of this extension:
https://addons.mozilla.org/en-US/firefox/addon/about-sessionstore/

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

Tagging with memshrink because of the issues referred to, especially in comment 8 and comment 9.

Comment 11

[Nicholas Nethercote [inactive]]

What exactly are the issues?  Do we have any measurements indicating the memory consumption is excessive?

Comment 12

[Emanuel Hoogeveen [:ehoogeveen]]

In bug 942601 I saw bursts of 400-500 MiB RAM usage in Windows' task manager each time the sessionstore file was updated, and saw stable memory usage decrease by ~100 MiB after opening the pages in new tabs and clearing the old entries. Note that bug 943339 should mitigate my issue a lot, though there may be other situations that need a bigger hammer (or a subtler one).

Comment 13

[Randell Jesup [:jesup] (needinfo me)]

400MB for a 26MB file is a regression from when I filed the initial sessionstore bugs (669603, etc)  Back then, I'd see a spike of circa 4-5x the sessionstore.js size.  Some large expansion was (is?) unavoidable due to having to convert it entirely to a JSON object and then write it; several proposals were on the table about how to alleviate the problem and do incremental writes.

Comment 14

[Emanuel Hoogeveen [:ehoogeveen]]

Note I haven't had a chance to test this in a fresh profile yet, so if addons can make this worse, they probably are. In addition I need to test and see if bug 930967 made a difference.

Comment 15

[Tim Taubert [:ttaubert] (inactive)]

Bug 952998 changed sessionstore to only collect DOMSessionStorage data for the currently shown history entry. Data for other shistory entries is not collected. This should reduce data collection time and also sessionstore.js file size.

Comment 16

[Randell Jesup [:jesup] (needinfo me)]

Ping as IIRC you've been looking at sessionstore stuff lately. While most details here are now irrelevant, we still have the original behavior. Also I wonder how much we spike memory use during a save of session data.

Comment 17

[BugBot [:suhaib / :marco/ :calixte]]

In the process of migrating remaining bugs to the new severity system, the severity for this bug cannot be automatically determined. Please retriage this bug using the new severity system.

Comment 18

[Andreas Farre [:farre]]

Yeah, not at all sure of how relevant this is. Dao, any ideas?