Closed Bug 937651 Opened 6 years ago Closed 6 years ago

Replace the sessionstore.js with an sessionstore.sqlite

Categories

(Firefox :: Session Restore, defect)

defect
Not set

Tracking

()

RESOLVED WONTFIX

People

(Reporter: BesTo, Unassigned)

References

(Blocks 1 open bug, )

Details

Crash Data

It seems that when a user always saves his session and restart them, the sessionstore.js grows rapidly, Firefox needs a lot of memory, slow down, get instable and creates a lot of crashers (with no thread identified).
Replacing the sessionstore.js with an sessionstore.sqlite can maybe fix this (and a lot of other) problems.


It seems that when a user always saves his session and restart them (e.g. after crashes) - even he do not reload all tabs - the sessionstore.js grows rapidly.
It seems that then Firefox needs more and more memory from session to session, slow down and get instable.
It seems that FF crashes then a lot - mostly with:
[@ EMPTY: no crashing thread identified; corrupt dump ]

If the sessionstore.js reaches ~100MB it seems FF crashes immediately after restart with the following two Crash Signatures:
[@ mozalloc_abort(char const* const) | NS_DebugBreak | nsTextEditorState::GetValue(nsAString_internal&, bool) ]
[@ mozalloc_abort(char const* const) | mozalloc_handle_oom(unsigned int) | moz_xmalloc | ConvertBreaks<wchar_t> ]

Also a big sessionstore.js file can't be opened in any other program to work with the session for e.g. convert it to bookmarks.


Replacing the sessionstore.js with an sessionstore.sqlite can maybe fix this (and a lot of other) problems.


Problems that maybe can fixed in this way, too:
Bug 394492 - SessionStore API performance issues with large number of windows and tabs
https://bugzilla.mozilla.org/show_bug.cgi?id=394492
Bug 511135 - crash in AppendUTF8toUTF16 during startup due to large sessionstore.js
https://bugzilla.mozilla.org/show_bug.cgi?id=511135
Bug 532810 - sessionstore screwing over our dromaeo performance
https://bugzilla.mozilla.org/show_bug.cgi?id=532810
Bug 539597 - Make SessionStore handle full screen mode
https://bugzilla.mozilla.org/show_bug.cgi?id=539597
Bug 550967 - SessionStore non-trivially slows down startup
https://bugzilla.mozilla.org/show_bug.cgi?id=539597
Bug 581510 - user is not warned in firefox UI that sessionstore protection has stopped updating due to memory shortage, error console "out of memory ... nsSessionStore.js Line: 2869"
https://bugzilla.mozilla.org/show_bug.cgi?id=581510
Bug 628870 - Recursive "#sessionData" saving can result in enormous sessionstore.js
https://bugzilla.mozilla.org/show_bug.cgi?id=628870
Bug 641164 - sessionrestore OOM crash on startup when hundreds of tabs and large sessionstore.js
https://bugzilla.mozilla.org/show_bug.cgi?id=641164
Bug 666584 - SessionStore should send incremental changes instead of complete history
https://bugzilla.mozilla.org/show_bug.cgi?id=666584
Bug 722051 - Sessionstore.js being truncated after restart
https://bugzilla.mozilla.org/show_bug.cgi?id=722051
Bug 810932 - Investigate how to redesign sessionstore.js for improved performance
https://bugzilla.mozilla.org/show_bug.cgi?id=810932
Bug 903621 - sessionstore protection lost. NO "out of memory" warning/error
https://bugzilla.mozilla.org/show_bug.cgi?id=903621
Additional Infos:
I worked the last weeks with no plugins and and the NoScript Add-on.
I submitted from 2013-08-20 till 2013-11-11 40 crash reports - most of them "no thread identified".

This bug can maybe fixed with this redesign, too:
Bug 934935 - Facebook.com spams sessionstore.js
Also: Bug 669034 - (sessionRestoreJank) [meta] Re-architect session restore to avoid periodic freezes
The stack extracts that you mention seem to hint that the problem is not in JSON [de]serialization. While we can certainly improve the storage format, I'm almost sure that moving to sqlite would not resolve these stack crashes.

Also, we have been studying the sessionrestore-related slowdowns, and we have not reached yet the stage at which the JSON part of the operations is the dominant cost. We will eventually reach that stage, mind you, once we have finished optimizing the data collection through the use of caching + e10s. Once that is done, the main JSON-related cost will be the communication between the main thread and the worker thread, which we already know how to optimize aggressively (bug 886447).

Now, as I mention, our storage format is not particularly great. The main advantage of JSON is its simplicity. It is certainly possible to design something better. It is possible that sqlite would be better whenever the file is huge, but I'm not sure of that.

For the moment, the dominant cost is (still) data collection, which is totally independent from the storage backend. Once we have finished optimizing this, we will start optimizing communication between the main thread and the I/O thread. This
(In reply to David Rajchenbach Teller [:Yoric] <needinfo? me> from comment #3)
> For the moment, the dominant cost is (still) data collection, which is
> totally independent from the storage backend. Once we have finished
> optimizing this, we will start optimizing communication between the main
> thread and the I/O thread. This

Playing devil's advocate here. You could look at the problem from a different angle: why do you need to do data collection? Because the storage backend requires it. With a database type storage, you could do without data collection, and just run database update queries as the changes occur (and could do that to memory tables, you would then commit to disk, if you're worried about the I/O cost)
(In reply to Mike Hommey [:glandium] from comment #4)
> Playing devil's advocate here. You could look at the problem from a
> different angle: why do you need to do data collection? Because the storage
> backend requires it. With a database type storage, you could do without data
> collection, and just run database update queries as the changes occur (and
> could do that to memory tables, you would then commit to disk, if you're
> worried about the I/O cost)

Actually, no, we are doing data collection because it's easier. But we are currently refactoring away from data collection as part of bug 930967. Once this is done we can start thinking of improved back-ends.
The problem with a SQLite file is that it would be even less human-readable than the current JavaScript file. A nice storage would be a XML file, like OmniWeb does. It would be a step towards interoperability.
I'm not sure XML would really change anything to the problems we have.
Thanks for the suggestion, but as Yoric explained this isn't something we'd consider at this point, since it would be complicated to implement, wouldn't fix the most serious problems with sessionstore, and has several other downsides (like the one mentioned in comment 6).
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
(In reply to :Gavin Sharp (email gavin@gavinsharp.com) from comment #8)
> Thanks for the suggestion, but as Yoric explained this isn't something we'd
> consider at this point, since it would be complicated to implement, wouldn't
> fix the most serious problems with sessionstore, and has several other
> downsides (like the one mentioned in comment 6).

As Mike Hommey [:glandium] in comment 4 wrote:
> Playing devil's advocate here. You could look at the problem from a different angle: why do you need 
> to do data collection? Because the storage backend requires it. With a database type storage, you 
> could do without data collection, and just run database update queries as the changes occur (and could 
> do that to memory tables, you would then commit to disk, if you're worried about the I/O cost)

And you should try to read/use/work with an sessionstore.jf file that have some MB ... it's all in one line and you wouldn't be able to import it 'the easy way' into something else (e.g.: LibreOffice) ...
For an SQLite File I can use an SQLite Editor.

In Bug 930967 Tim Taubert [:ttaubert] works on 'Let tabs broadcast data when it changes instead of collecting it through the main process' ...
I think it would be much better to just save the changed tabs in an DB instead to write the js files every time again.
Severity: major → enhancement
Status: RESOLVED → REOPENED
Depends on: 930967
OS: Windows 7 → All
Hardware: x86_64 → All
Resolution: WONTFIX → ---
Version: 26 Branch → unspecified
(In reply to Tobias B. Besemer from comment #9)
> And you should try to read/use/work with an sessionstore.jf file that have
> some MB ... it's all in one line and you wouldn't be able to import it 'the
> easy way' into something else (e.g.: LibreOffice) ...
> For an SQLite File I can use an SQLite Editor.

Do you have a specific application in mind for this usecase?

At the moment, I see the following reasons to not pursue this strategy:
- it will not solve any of the main issues that we are trying to fix;
- it would require considerable rewrite of the session restore code;
- it would break a number of add-ons;
- it would slow down the first load of session restore, hence the time it takes for Firefox to startup;
- it would also slow down a few other things (duration of backups, in particular).

While there would be advantages to using a sqlite database instead of a json file, our estimate is that, in the current setting, the advantages attained would be much lesser than the drawbacks.

However, we are looking at using sqlite for a subset of the session store, namely the DOM storage, which tends to use considerable space in sessionstore.js for bad reasons (see bug 669603, which you have commented already).
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → WONTFIX
(In reply to David Rajchenbach Teller [:Yoric] <needinfo? me> from comment #10)
> (In reply to Tobias B. Besemer from comment #9)
> > And you should try to read/use/work with an sessionstore.jf file that have
> > some MB ... it's all in one line and you wouldn't be able to import it 'the
> > easy way' into something else (e.g.: LibreOffice) ...
> > For an SQLite File I can use an SQLite Editor.
> 
> Do you have a specific application in mind for this usecase?

If a session comes no more up after a crash (maybe to much tabs and to less memory on the PC), all my open tabs are lost if I can't extract them from the sessionstore file.

Now it is nearly impossible to extract something usable.


> At the moment, I see the following reasons to not pursue this strategy:
> - it will not solve any of the main issues that we are trying to fix;

The idea behind was that a text file will be always saved complete.

So if the sessionstore.js have e.g. 50 MB, on every save Firefox will write 50 MB to the HD.
- This cost a lot of time.
- It needs a lot of power on Notebooks.
- It fragmentate my HD
- It shorten the life of an HD/SSD

This also happens when if have only changed one tab out of 500!

A database writes only the changes that happens.

So a db will only update this single tab.


> - it would require considerable rewrite of the session restore code;

This was the reason for me to set it as 'enhancement'.


> - it would break a number of add-ons;

That was never a reason for Mozilla to don't do things. ;-)


> - it would slow down the first load of session restore, hence the time it
> takes for Firefox to startup;

But after the restart - if it don't hangs and write all the time on my HD - it should be much faster.


> - it would also slow down a few other things (duration of backups, in
> particular).

How much bigger would be an SQLite file?
Or how much will it slow which component?

> While there would be advantages to using a sqlite database instead of a json
> file, our estimate is that, in the current setting, the advantages attained
> would be much lesser than the drawbacks.
> 
> However, we are looking at using sqlite for a subset of the session store,
> namely the DOM storage, which tends to use considerable space in
> sessionstore.js for bad reasons (see bug 669603, which you have commented
> already).

I was flying over bug 669603 but don't find the part with using SQLite ...
Is there a special bug for this that I can monitor ???

I don't reopen this bug again because it looks like wikipedia edit wars ... ;-)
But I don't think that bugs must be so fast marked as 'won't fix'! ;-)
Flags: needinfo?(dteller)
(In reply to Tobias B. Besemer from comment #11)
> If a session comes no more up after a crash (maybe to much tabs and to less
> memory on the PC), all my open tabs are lost if I can't extract them from
> the sessionstore file.
> 
> Now it is nearly impossible to extract something usable.

I am not convinced that it would be easier with a sqlite database. Additionally, I am sure that only a few hundred users would ever try it, out of our .5billion users, and that suggests that this argument is not compelling enough, especially given the fact that it's a very sophisticated refactoring.

> > At the moment, I see the following reasons to not pursue this strategy:
> > - it will not solve any of the main issues that we are trying to fix;
> 
> The idea behind was that a text file will be always saved complete.
> 
> So if the sessionstore.js have e.g. 50 MB, on every save Firefox will write
> 50 MB to the HD.
> - This cost a lot of time.
> - It needs a lot of power on Notebooks.
> - It fragmentate my HD
> - It shorten the life of an HD/SSD
> 
> This also happens when if have only changed one tab out of 500!
> 
> A database writes only the changes that happens.
> 
> So a db will only update this single tab.

This is mostly true. While we do not have a clear indication of the size of sessionstore.js at the moment (see bug 942063), I suspect that for most users, it is less than 300kb. For files of this size, my (admittedly limited) experience suggests that a sqlite file is larger, slower to read/write and requires more I/O.

> > - it would require considerable rewrite of the session restore code;
> 
> This was the reason for me to set it as 'enhancement'.

Sure.

> > - it would break a number of add-ons;
> 
> That was never a reason for Mozilla to don't do things. ;-)

Actually, we are taking lots of effort to not break add-ons. We only break them when we don't see an alternative.

> > - it would slow down the first load of session restore, hence the time it
> > takes for Firefox to startup;
> 
> But after the restart - if it don't hangs and write all the time on my HD -
> it should be much faster.

Well, it *may* be faster. That's yet to be determined.

> > - it would also slow down a few other things (duration of backups, in
> > particular).
> 
> How much bigger would be an SQLite file?
> Or how much will it slow which component?

I have no clear idea. However, if you are interested in contributing to this bug, and if you have time to write a sessionstore.js -> sessionstore.sqlite converter (for some arbitrary sqlite table structure), it would be an interesting information. This doesn't mean that we'll do it, but at least, we'll be able to take more informed decisions.

> > While there would be advantages to using a sqlite database instead of a json
> > file, our estimate is that, in the current setting, the advantages attained
> > would be much lesser than the drawbacks.
> > 
> > However, we are looking at using sqlite for a subset of the session store,
> > namely the DOM storage, which tends to use considerable space in
> > sessionstore.js for bad reasons (see bug 669603, which you have commented
> > already).
> 
> I was flying over bug 669603 but don't find the part with using SQLite ...
> Is there a special bug for this that I can monitor ???

Ah, my bad, I seemed to remember that it was sqlite-based, but it isn't. Doing this with sqlite is still an option, though :)

> I don't reopen this bug again because it looks like wikipedia edit wars ...
> ;-)
> But I don't think that bugs must be so fast marked as 'won't fix'! ;-)

Thanks :)
Flags: needinfo?(dteller)
(In reply to David Rajchenbach Teller [:Yoric] <needinfo? me> from comment #12)
> ... of our .5billion users, and that suggests that this argument is not ...
5 billion ??? ;-)

> This is mostly true. While we do not have a clear indication of the size of
> sessionstore.js at the moment (see bug 942063), I suspect that for most
> users, it is less than 300kb.
I don't think so !!! :D
Please look at Bug 934934.
(In reply to Tobias B. Besemer from comment #13)
> (In reply to David Rajchenbach Teller [:Yoric] <needinfo? me> from comment
> #12)
> > ... of our .5billion users, and that suggests that this argument is not ...
> 5 billion ??? ;-)

There's a dot, so it's ~500 million users (außerdem: billion == Milliarden).

(In reply to Tobias B. Besemer from comment #13)
> I don't think so !!! :D
> Please look at Bug 934934.

Then prove it (Bug 942063). Singular evidence does not scale to millions of users very well. 

Either way, the benefits of such a rewrite would not outweigh the cost. At least for now, this is WONTFIX, we'll see what happens some time in the future.
While I am not convinced by the above arguments, it might well be that a sessionstore.sqlite would be much more memory-efficient than sessionstore.js. This looks difficult to measure, but it might be worth investigating once we have finished rewriting SessionStore.jsm to broadcast changes.
(In reply to David Rajchenbach Teller [:Yoric] (away 20 Dec to Jan 4th - please use "needinfo?") from comment #12)
> This is mostly true. While we do not have a clear indication of the size of
> sessionstore.js at the moment (see bug 942063), I suspect that for most
> users, it is less than 300kb. For files of this size, my (admittedly
> limited) experience suggests that a sqlite file is larger, slower to
> read/write and requires more I/O.

Early Telemetry indicates that 75% percent of Nightly users have a sessionstore.js weighing 194kb or less and 95% of users have 1.2Mb or less:

http://telemetry.mozilla.org/#nightly/29/FX_SESSION_RESTORE_FILE_SIZE_BYTES
Severity: enhancement → normal
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Blocks: 810932
I'm pretty sure that our opinion on SQLite didn't change. It is much more likely to progress towards a journaled storage in the future.
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → WONTFIX
(In reply to Tim Taubert [:ttaubert] from comment #17)
> I'm pretty sure that our opinion on SQLite didn't change. It is much more
> likely to progress towards a journaled storage in the future.

I reopened this bug, because this solution is now mentioned as an option in the wiki ...
If you be sure, that this is no option, please update the wiki.
If you be not sure, please reopen the bug.

https://wiki.mozilla.org/Firefox/session_restore#Sqlite
Flags: needinfo?(ttaubert)
The wiki is just a collection of ideas, including some brainstorming. It is not an official document.
Flags: needinfo?(ttaubert)
(In reply to David Rajchenbach Teller [:Yoric] from comment #16)
> (In reply to David Rajchenbach Teller [:Yoric] (away 20 Dec to Jan 4th -
> please use "needinfo?") from comment #12)
> > This is mostly true. While we do not have a clear indication of the size of
> > sessionstore.js at the moment (see bug 942063), I suspect that for most
> > users, it is less than 300kb. For files of this size, my (admittedly
> > limited) experience suggests that a sqlite file is larger, slower to
> > read/write and requires more I/O.
> 
> Early Telemetry indicates that 75% percent of Nightly users have a
> sessionstore.js weighing 194kb or less and 95% of users have 1.2Mb or less:
> 
> http://telemetry.mozilla.org/#nightly/29/FX_SESSION_RESTORE_FILE_SIZE_BYTES


But I think you can't ignore the 100k+ users that have a ss.js >10MB.

http://telemetry.mozilla.org/#filter=release%2F29%2FFX_SESSION_RESTORE_FILE_SIZE_BYTES&aggregates=multiselect-all!Submissions!Mean!5th%20percentile!25th%20percentile!median!75th%20percentile!95th%20percentile&evoOver=Builds&locked=true&sanitize=true&renderhistogram=Table
A discussion about changes/improvements to the Session Restore (sessionstore):
https://groups.google.com/forum/#!topic/mozilla.dev.platform/JHrOP3yMgfg
You need to log in before you can comment on or make changes to this bug.