The most interesting things from this release: - WAL journaling can reduce fsyncs, we want it for Places and probably urlclassifier. It can help I/O. - query planner enhancements can speed up queries globally
forgot to add reasoning. This should be blocking because it goes to the right direction of I/O and database fragmentation reduction. and perf++. Removing our temp tables will improve statements performances but will increase fsyncs for visits, this should compensate for it plus giving wins on fragmentation (Taras discovered that also vacuum cannot fight gragmentation too well on highly written databases). It also has an enhancement for memory measure that could be useful for about:memory (heap memory used by cache, per connection). Risk is the usual for sqlite upgrades, they have nice tests and QA.
I'll work on this once it gets blocking status...
Created attachment 459508 [details] [diff] [review] Mozilla Changes
Created attachment 459510 [details] [diff] [review] SQLite Changes Oddly enough, this is small enough to upload...
If we're going to do this, we'll want it in the next beta so we can live with it for long enough that any unforseen problems will appear.
Pushed to the try server in changeset 64bbbfa74973
Try hated that one and did not build a thing. I pushed it again in changeset 1209077b6e8a
See also: https://bugzilla.mozilla.org/show_bug.cgi?id=581374 60 lines of C to add sqlite compression, shrinks db 50-80% at a low cycle cost.
Try looked OK, even if it only ran win2003 unit tests. I ran an experimental build before and it worked out just fine, so I don't expect any issues. Will land this once the tree greens up a bit.
http://hg.mozilla.org/mozilla-central/rev/7a3ecace6732 http://hg.mozilla.org/mozilla-central/rev/18127ac02bf2
This caused a "Regression: Ts Shutdown, MAX Dirty Profile increase 21.3% on Linux Firefox" according to the graphs bot.
(In reply to comment #11) > This caused a "Regression: Ts Shutdown, MAX Dirty Profile increase 21.3% on > Linux Firefox" according to the graphs bot. Right, and as commented on the threads for that, the benefits we'll get with enabling WAL on places should correct that. We've also had massive wins on that benchmark for Firefox 4.0, and last I checked we are still far ahead of Firefox 3.6.
(In reply to comment #12) > (In reply to comment #11) > > This caused a "Regression: Ts Shutdown, MAX Dirty Profile increase 21.3% on > > Linux Firefox" according to the graphs bot. > Right, and as commented on the threads for that, the benefits we'll get with > enabling WAL on places should correct that. We've also had massive wins on > that benchmark for Firefox 4.0, and last I checked we are still far ahead of > Firefox 3.6. Where are those threads? I'm not sure where to look, but I can only find http://groups.google.co.uk/group/mozilla.dev.tree-management/browse_thread/thread/9c4a31c6eb105aca?hl=en#
Weird. I know I responded to that, but I don't see it anywhere. :(
It's unclear to me from comment 12 whether the regression was expected or not, and if so, why. If it wasn't, then regardless of whether bug 573492 cancels out the difference or not, shouldn't it be looked into?
It is not expected, but the argument I am making is that the benefits outweigh the costs here.
This just got backed out. People are seeing 3.7.0 reporting some databases as corrupted when running queries on them, but then going back to 3.6.22 and not seeing these same errors. I've sent one database to the SQLite team to take a look. This may end up being WONTFIXed, or we might reland. Results pending. http://hg.mozilla.org/mozilla-central/rev/e5e6583bd137 (also no reason to block beta 3 on this; it just needs beta coverage although beltzner may disagree)
New blocking status is fine, thanks for letting us (and more importantly laura mesa!) know.
places.sqlite was one that got badly corrupted on my machine that i finally deleted it per advise from others, and after that the browser has appeared to be ok. Didn't think to try to save it though.
It looks like the corruption isn't actually corruption. The file size stored in the database header is wrong in the one database file I sent to the SQLite team. As a result, 3.7.0 was missing pages from the end of the file, causing the corrupt error messages. The databases should work fine with SQLite 3.6.23.1. Not WONTFIXing yet until we learn more, but I suspect that that will be the case here.
Basically for me, History was not working properly. The UI didn't work the way it should if there was no problem. For example, I would open a web page from session restore, click on link to the new page, and there was no page to go back to as the back button turned into the disabled state and history didn't record itself in the history list.
So, we'll want to take 3.7.0.1 which has a fix for the corruption issue. Details here: http://www.sqlite.org/src/info/51ae9cad31
*** Bug 581571 has been marked as a duplicate of this bug. ***
Created attachment 463189 [details] [diff] [review] Mozilla Changes
Created attachment 463190 [details] [diff] [review] SQLite Changes
Do we have tests for the corruption issue that we caught so that if it happens again we can catch it again before landing?
(In reply to comment #26) > Do we have tests for the corruption issue that we caught so that if it happens > again we can catch it again before landing? The SQLite guys have tests for this now (and they run their full test suite before releasing), but in order for us to test it, I'd have to have a copy of SQLite before 3.7.0 in the tree and one after, and build both, and then have them both write to the database.
This time, it appears to have landed without any performance regressions (which is likely fixed by the query plan optimization bug they mention in the list of changes). http://hg.mozilla.org/mozilla-central/rev/280b993408e7 http://hg.mozilla.org/mozilla-central/rev/b840177c952a
Hmm, still have Ts Shutdown regressions of about the same size. Again though, we are still under Firefox 3.6, and have other fixes in the work that depend on this that will improve the shutdown (and startup, and Tp) time.
(In reply to comment #29) > Hmm, still have Ts Shutdown regressions of about the same size. Again though, > we are still under Firefox 3.6, and have other fixes in the work that depend on > this that will improve the shutdown (and startup, and Tp) time. Sounds fine to me.
Backed out this bug's changesets, for causing bug 583821. backout: http://hg.mozilla.org/mozilla-central/rev/431901d7a495 merge: http://hg.mozilla.org/mozilla-central/rev/bf982bac3ccd backout: http://hg.mozilla.org/mozilla-central/rev/f069efb502f6 merge: http://hg.mozilla.org/mozilla-central/rev/a9b21fa2b7ab See in particular bug 583821 comment 36 and bug 583821 comment 40. Looks like we need to wait for SQLite 3.7.1.
Thanks a ton!
We are not going to take this fix now, and will wait for SQLite 3.7.1 due to comment 31. Will carry over blocking flags to there.
FWIW, the tbpl cycle on this backout had a test-failure due to a storage-related NS_ABORT_IF_FALSE failure, on a Win2003 Md Oth cycle. The NS_ABORT_IF_FALSE failure was: { ###!!! ABORT: Getting value failed, wrong column index?: 'NS_SUCCEEDED(rv)', file e:\builds\moz2_slave\mozilla-central-win32-debug\build\obj-firefox\dist\include\mozIStorageStatement.h, line 320 } Log is: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1282016084.1282018131.8619.gz WINNT 5.2 mozilla-central debug test mochitest-other on 2010/08/16 20:34:44 Comment 31 was just a clean backout of this bug's changesets, in reverse order (hg backout b840177c952a, hg merge, hg commit; repeat for changeset 280b993408e7). So I don't think it was at fault. Feel free to audit it if you suspect it, though.
The ABORT_IF_FALSE from comment 34 is actually preceeded by this (non-fatal) assertion (visible in full log but not in shortlog): ###!!! ASSERTION: hasResult is false but the call succeeded?: 'hasResult', file e:/builds/moz2_slave/mozilla-central-win32-debug/build/toolkit/components/places/src/nsNavHistory.cpp, line 1941
(In reply to comment #35) > The ABORT_IF_FALSE from comment 34 is actually preceeded by this (non-fatal) > assertion (visible in full log but not in shortlog): > ###!!! ASSERTION: hasResult is false but the call succeeded?: 'hasResult', file > e:/builds/moz2_slave/mozilla-central-win32-debug/build/toolkit/components/places/src/nsNavHistory.cpp, > line 1941 I believe that is a known intermittent failure.
Yeah -- in any case, it went green later, so it looks like it was just a particularly coincidental intermittent orange. Sorry for the bug-spam -- disregard comment 34 through this one.
or
