The problem is that without temp tables we have more fsyncs, not a lot more because we have made our best to reduce them, and those are also on the background thread. But we still fight the lovely Linux filesystem. The idea is to use WAL to reproduce the temp tables behavior, i.e. get temp tables advantages without having their disadvantages. In WAL mode with synchronous full any single transaction is committed to the WAL with a fsync, then on specified times (size constraints really) the journal is committed to the main database with a fsync. This differs from before since writing to temp tables was avoiding the fsyncs (not always though, for bookmarks we were still forcing flushes). Changing our synchronous mode to normal (the Storage default), will not fsync anymore commits to the WAL, but checkpoints to the main database will still be fsynced. This means we can have almost (disk is still slower than memory) the same speed as temp tables, since we don't fsync every single transaction or write. Writes will be really cheap. Talking about data safety, the main database cannot even corrupt, because the wal is checkpointed to it with fsyncs (we could also evaluate to enable fullfsyncs on Mac). The worst case scenario is that after a crash the full WAL gets lost (rather than just the last transaction), and with it we lose all the last transactions. This is partly similar to a crash with not-yet-flushed temp tables, probably better because we lose full transactions rather then partial transactions with partial data. Ways to mitigate data safety issues are: - use a small enough WAL, so that eventual size of lost data is small - force checkpoints at strategic points

I tried to skip the methods that are not that interesting from a dataloss point of view, like insertSeparator or setting a lastModified

Comment on attachment 494865 [details] [diff] [review] part 1: normal synchronous wal >+++ b/toolkit/components/places/src/nsNavHistory.cpp >+#define DATABASE_MAX_WAL_SIZE_IN_BYTES 524288 I would prefer this to be DATABASE_MAX_WAL_SIZE_IN_KILOBYTES as that is easier to reason with than bytes. Just do the math at first use please. r=sdwilsh with that fixed.

looks like I forgot to save Helpers.h, that forceWALCheckpoint should be uppercase. fixed locally.

Comment on attachment 494888 [details] [diff] [review] part 2: force checkpoints in bookmarks >+++ b/toolkit/components/places/src/Helpers.cpp >+void >+ForceWALCheckpoint(mozIStorageConnection* aDBConn) >+{ >+ nsCOMPtr<mozIStorageAsyncStatement> stmt; >+ aDBConn->CreateAsyncStatement(NS_LITERAL_CSTRING( nit: (void) >+ NS_ASSERTION(stmt, "Should be possible to force a WAL checkpoint"); >+ if (stmt) { This should only fail in out of memory situations, which would end up aborting anyway. Lose the assertion, and the null check. >+++ b/toolkit/components/places/src/Helpers.h >+/** >+ * Forces a WAL checkpoint. >+ * This will cause all transactions stored in the journal file to be committed nit: lose the newline between these two lines >+ * @param aDBConn >+ * Connection to the database. >+ * @note The checkpoint will force a fsync. nit: "force an fsync/Flush". Let's start to include the windows method name too ;) >+void >+forceWALCheckpoint(mozIStorageConnection* aDBConn); nit: in .h files, we keep the return type on the same line. r=sdwilsh

part 1: http://hg.mozilla.org/projects/places/rev/9d57c1a44480 part 2: http://hg.mozilla.org/projects/places/rev/86b924f61b01

http://hg.mozilla.org/mozilla-central/rev/9d57c1a44480 http://hg.mozilla.org/mozilla-central/rev/86b924f61b01