Last Comment Bug 558646 - Port Bug 506482 [Don't write sessionstore.js to disk for read only events] to SeaMonkey
: Port Bug 506482 [Don't write sessionstore.js to disk for read only events] to...
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Session Restore (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.1a2
Assigned To: Misak Khachatryan
:
:
Mentors:
Depends on: 538672 506482
Blocks:
  Show dependency treegraph
 
Reported: 2010-04-11 04:23 PDT by Misak Khachatryan
Modified: 2010-05-19 10:34 PDT (History)
0 users
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch plus test (7.29 KB, patch)
2010-04-11 04:47 PDT, Misak Khachatryan
no flags Details | Diff | Splinter Review
patch plus test (disabled) (8.12 KB, patch)
2010-05-18 23:44 PDT, Misak Khachatryan
neil: review+
neil: superreview+
Details | Diff | Splinter Review
patch without test (checked in) (3.30 KB, patch)
2010-05-19 10:24 PDT, Misak Khachatryan
misak.bugzilla: review+
misak.bugzilla: superreview+
Details | Diff | Splinter Review

Description Misak Khachatryan 2010-04-11 04:23:39 PDT
From parent bug:

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.1)
Gecko/20090715 Firefox/3.5.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.1)
Gecko/20090715 Firefox/3.5.1

Although I appreciate that session restore really does restore every detail of
my session, this becomes unwieldy when sessionstore.js gets large. With just 61
tabs across 3 windows, my sessionstore.js is nearly 2MB. This results in over
half a gigabyte of disk writes after an hour of browsing!

This 2MB file is rewritten in its entirety whenever:

1. I open or close a tab (obviously)
2. I click a link (since session restore remembers every tab's history)
3. I switch to another tab (since it remembers the currently open tab)
4. I scroll within a tab (since it remembers where I am in the page!)

Therefore, despite the 10-second hysteresis in rewriting this file, an active
browsing session means that it probably will get rewritten every 10 seconds. At
2MB a pop, that's 720MB per hour.

On my older machines, this is a very noticeable performace hit, especially when
opening new tabs; on my laptops, this wastes power; on my SSDs, it wastes flash
cycles.

Reproducible: Always

Steps to Reproduce:
1. Open many tabs until sessionstore.js gets quite large. (I am not sure why
some pages grow sessionstore.js by a few kilobytes and other pages grow it by
hundreds. As a rule of thumb, opening every article on the front page of
CNN.com will get you close to 2MB.)

2. Open task manager (or equivalent) and watch the total disk writes generated
by the Firefox process. Or use procmon (or equivalent) and watch Firefox
rewrite sessionstore.js.

3. Observe rewrites occurring whenever a tab is opened, a link is clicked, a
tab is selected, or a page is scrolled. Observe Firefox write multiple
gigabytes after a mere evening's worth of ordinary browsing.
Actual Results:  
For my usage patterns, I see Firefox writing 2-3GB per day, sometimes more. The
vast majority of this is sessionstore activity, with sqllite contributing
comparatively little.

Expected Results:  
The session should be stored in a way that's amenable to partial updates.
Throwing it into sqllite is probably overkill. Would it be difficult to write
session activity as a log, and rewrite it only occasionally?

Some of the updates are extremely small.  Here are the diffs I get when I click
on a new tab:

< {"state":"running","lastUpdate":1248521427588}})
--
> {"state":"running","lastUpdate":1248521292034}})

And here are the diffs I get when I scroll within an existing tab:

< {"state":"running","lastUpdate":1248521486237}})
--
> {"state":"running","lastUpdate":1248521427588}})

So in the worst-case scenario, where I'm not opening any tabs but simply
*reading for an hour*, I could conceivably generate 720MB of disk writes to
update as little as 1800 bytes of data (or 1.4MB, using 4k clusters). And all
that just to remember where I've scrolled to in the page.

At the least, please add an option to tweak the behavior.

This is a fresh install of Firefox 3.5.1 on a fresh install of XP SP3 with the
latest patches. The problem existed before I installed any extensions.
Comment 1 Misak Khachatryan 2010-04-11 04:47:06 PDT
Created attachment 438348 [details] [diff] [review]
patch plus test

Test passing on my Fedora 12, but there is known issue https://bugzilla.mozilla.org/show_bug.cgi?id=538672 , also i saw some test sometimes fail with this patch:

TEST-INFO | (browser-test.js) | Waiting for window activation...
Running chrome://mochikit/content/browser/suite/common/tests/browser/browser_524745.js...
TEST-PASS | chrome://mochikit/content/browser/suite/common/tests/browser/browser_524745.js | Only one browser window should be open initially
TEST-PASS | chrome://mochikit/content/browser/suite/common/tests/browser/browser_524745.js | before wait for focus -- loaded: complete active window: ([object ChromeWindow]) chrome://navigator/content/navigator.xul focused window: <no window focused> desired window: ([object ChromeWindow]) chrome://navigator/content/navigator.xul docshell visible: true
TEST-PASS | chrome://mochikit/content/browser/suite/common/tests/browser/browser_524745.js | must wait for focus
TEST-PASS | chrome://mochikit/content/browser/suite/common/tests/browser/browser_524745.js | maybe run tests <load:true, focus:true> -- loaded: complete active window: ([object ChromeWindow]) chrome://navigator/content/navigator.xul focused window: ([object ChromeWindow]) chrome://navigator/content/navigator.xul desired window: ([object ChromeWindow]) chrome://navigator/content/navigator.xul docshell visible: true
TEST-PASS | chrome://mochikit/content/browser/suite/common/tests/browser/browser_524745.js | MozAfterPaint event received
TEST-PASS | chrome://mochikit/content/browser/suite/common/tests/browser/browser_524745.js | maybe run tests <load:true, focus:true> -- loaded: complete active window: ([object ChromeWindow]) chrome://navigator/content/navigator.xul focused window: ([object XPCNativeWrapper [object Window]]) about:blank desired window: ([object ChromeWindow]) chrome://navigator/content/navigator.xul docshell visible: true
TEST-PASS | chrome://mochikit/content/browser/suite/common/tests/browser/browser_524745.js | selectedWindow is window_B
TEST-PASS | chrome://mochikit/content/browser/suite/common/tests/browser/browser_524745.js | before wait for focus -- loaded: complete active window: ([object ChromeWindow]) chrome://navigator/content/navigator.xul focused window: ([object XPCNativeWrapper [object Window]]) about:blank desired window: ([object XPCNativeWrapper [object Window]]) about:blank docshell visible: false
TEST-PASS | chrome://mochikit/content/browser/suite/common/tests/browser/browser_524745.js | must wait for focus
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/suite/common/tests/browser/browser_524745.js | Timed out
TEST-INFO | checking window state
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/suite/common/tests/browser/browser_524745.js | Found an unexpected browser window at the end of test run


When i removing test and leaving only patch, all tests passing. It seems test is problematic. I'll wait until issue will be solved, then request reviews.
Comment 2 Misak Khachatryan 2010-05-18 23:44:37 PDT
Created attachment 446160 [details] [diff] [review]
patch plus test (disabled)

The test is disabled on firefox side long time ago, but patch checked in, i want to do the same. Once the test will be fixed I'll update it accordingly.
Comment 3 neil@parkwaycc.co.uk 2010-05-19 09:12:22 PDT
Comment on attachment 446160 [details] [diff] [review]
patch plus test (disabled)

OK but don't bother checking in the test until we have a working one.
Comment 4 Misak Khachatryan 2010-05-19 10:24:14 PDT
Created attachment 446244 [details] [diff] [review]
patch without test (checked in)
Comment 5 Misak Khachatryan 2010-05-19 10:33:46 PDT
Pushed: http://hg.mozilla.org/comm-central/rev/a83716817659

Forgot to mention - carrying forward r+ and sr+ from Neil

Note You need to log in before you can comment on or make changes to this bug.