diff options
author | shess <shess@chromium.org> | 2015-09-23 13:29:44 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-09-23 20:33:56 +0000 |
commit | 9a1948a4d6d445d5c8e209bdcd1cd050af72060b (patch) | |
tree | a2b6abac4673affa109bcc57ae72d67904ae8c29 /sql | |
parent | fa2fd3809241aae99bf07463ff99ad03dc9a1e07 (diff) | |
download | chromium_src-9a1948a4d6d445d5c8e209bdcd1cd050af72060b.zip chromium_src-9a1948a4d6d445d5c8e209bdcd1cd050af72060b.tar.gz chromium_src-9a1948a4d6d445d5c8e209bdcd1cd050af72060b.tar.bz2 |
[sql] Use memory-mapped I/O for sql::Connection.
sql::Connection::Open*() uses PRAGMA mmap_size to enable SQLite's
memory-mapped I/O. Additionally instrument to flush dirty pages from
the page cache after writes.
BUG=489784,533682
Review URL: https://codereview.chromium.org/1349863003
Cr-Commit-Position: refs/heads/master@{#350362}
Diffstat (limited to 'sql')
-rw-r--r-- | sql/connection.cc | 91 | ||||
-rw-r--r-- | sql/connection.h | 20 | ||||
-rw-r--r-- | sql/connection_unittest.cc | 78 | ||||
-rw-r--r-- | sql/statement.cc | 5 |
4 files changed, 194 insertions, 0 deletions
diff --git a/sql/connection.cc b/sql/connection.cc index 300e5e6..0b15387 100644 --- a/sql/connection.cc +++ b/sql/connection.cc @@ -466,6 +466,73 @@ void Connection::Preload() { } } +// SQLite keeps unused pages associated with a connection in a cache. It asks +// the cache for pages by an id, and if the page is present and the database is +// unchanged, it considers the content of the page valid and doesn't read it +// from disk. When memory-mapped I/O is enabled, on read SQLite uses page +// structures created from the memory map data before consulting the cache. On +// write SQLite creates a new in-memory page structure, copies the data from the +// memory map, and later writes it, releasing the updated page back to the +// cache. +// +// This means that in memory-mapped mode, the contents of the cached pages are +// not re-used for reads, but they are re-used for writes if the re-written page +// is still in the cache. The implementation of sqlite3_db_release_memory() as +// of SQLite 3.8.7.4 frees all pages from pcaches associated with the +// connection, so it should free these pages. +// +// Unfortunately, the zero page is also freed. That page is never accessed +// using memory-mapped I/O, and the cached copy can be re-used after verifying +// the file change counter on disk. Also, fresh pages from cache receive some +// pager-level initialization before they can be used. Since the information +// involved will immediately be accessed in various ways, it is unclear if the +// additional overhead is material, or just moving processor cache effects +// around. +// +// TODO(shess): It would be better to release the pages immediately when they +// are no longer needed. This would basically happen after SQLite commits a +// transaction. I had implemented a pcache wrapper to do this, but it involved +// layering violations, and it had to be setup before any other sqlite call, +// which was brittle. Also, for large files it would actually make sense to +// maintain the existing pcache behavior for blocks past the memory-mapped +// segment. I think drh would accept a reasonable implementation of the overall +// concept for upstreaming to SQLite core. +// +// TODO(shess): Another possibility would be to set the cache size small, which +// would keep the zero page around, plus some pre-initialized pages, and SQLite +// can manage things. The downside is that updates larger than the cache would +// spill to the journal. That could be compensated by setting cache_spill to +// false. The downside then is that it allows open-ended use of memory for +// large transactions. +// +// TODO(shess): The TrimMemory() trick of bouncing the cache size would also +// work. There could be two prepared statements, one for cache_size=1 one for +// cache_size=goal. +void Connection::ReleaseCacheMemoryIfNeeded(bool implicit_change_performed) { + // If memory-mapping is not enabled, the page cache helps performance. + if (!mmap_enabled_) + return; + + // On caller request, force the change comparison to fail. Done before the + // transaction-nesting test so that the signal can carry to transaction + // commit. + if (implicit_change_performed) + --total_changes_at_last_release_; + + // Cached pages may be re-used within the same transaction. + if (transaction_nesting()) + return; + + // If no changes have been made, skip flushing. This allows the first page of + // the database to remain in cache across multiple reads. + const int total_changes = sqlite3_total_changes(db_); + if (total_changes == total_changes_at_last_release_) + return; + + total_changes_at_last_release_ = total_changes; + sqlite3_db_release_memory(db_); +} + void Connection::TrimMemory(bool aggressively) { if (!db_) return; @@ -776,6 +843,9 @@ bool Connection::CommitTransaction() { RecordCommitTime(delta); RecordOneEvent(EVENT_COMMIT); + // Release dirty cache pages after the transaction closes. + ReleaseCacheMemoryIfNeeded(false); + return ret; } @@ -866,6 +936,12 @@ int Connection::ExecuteAndReturnErrorCode(const char* sql) { const base::TimeDelta delta = Now() - before; RecordTimeAndChanges(delta, read_only); } + + // Most calls to Execute() modify the database. The main exceptions would be + // calls such as CREATE TABLE IF NOT EXISTS which could modify the database + // but sometimes don't. + ReleaseCacheMemoryIfNeeded(true); + return rc; } @@ -1230,6 +1306,18 @@ bool Connection::OpenInternal(const std::string& file_name, // secure_delete. ignore_result(Execute("PRAGMA journal_mode = TRUNCATE")); + // Enable memory-mapped access. This value will be capped by + // SQLITE_MAX_MMAP_SIZE, which could be different between 32-bit and 64-bit + // platforms. + mmap_enabled_ = false; + if (!mmap_disabled_) + ignore_result(Execute("PRAGMA mmap_size = 268435456")); // 256MB. + { + Statement s(GetUniqueStatement("PRAGMA mmap_size")); + if (s.Step() && s.ColumnInt64(0) > 0) + mmap_enabled_ = true; + } + const base::TimeDelta kBusyTimeout = base::TimeDelta::FromSeconds(kBusyTimeoutSeconds); @@ -1273,6 +1361,9 @@ void Connection::DoRollback() { RecordUpdateTime(delta); RecordOneEvent(EVENT_ROLLBACK); + // The cache may have been accumulating dirty pages for commit. + ReleaseCacheMemoryIfNeeded(false); + needs_rollback_ = false; } diff --git a/sql/connection.h b/sql/connection.h index 19592d9..d456b6c 100644 --- a/sql/connection.h +++ b/sql/connection.h @@ -146,6 +146,9 @@ class SQL_EXPORT Connection { // other platforms. void set_restrict_to_user() { restrict_to_user_ = true; } + // Call to opt out of memory-mapped file I/O. + void set_mmap_disabled() { mmap_disabled_ = true; } + // Set an error-handling callback. On errors, the error number (and // statement, if available) will be passed to the callback. // @@ -638,6 +641,12 @@ class SQL_EXPORT Connection { return clock_->Now(); } + // Release page-cache memory if memory-mapped I/O is enabled and the database + // was changed. Passing true for |implicit_change_performed| allows + // overriding the change detection for cases like DDL (CREATE, DROP, etc), + // which do not participate in the total-rows-changed tracking. + void ReleaseCacheMemoryIfNeeded(bool implicit_change_performed); + // The actual sqlite database. Will be NULL before Init has been called or if // Init resulted in an error. sqlite3* db_; @@ -679,6 +688,17 @@ class SQL_EXPORT Connection { // databases. bool poisoned_; + // |true| if SQLite memory-mapped I/O is not desired for this connection. + bool mmap_disabled_; + + // |true| if SQLite memory-mapped I/O was enabled for this connection. + // Used by ReleaseCacheMemoryIfNeeded(). + bool mmap_enabled_; + + // Used by ReleaseCacheMemoryIfNeeded() to track if new changes have happened + // since memory was last released. + int total_changes_at_last_release_; + ErrorCallback error_callback_; // Tag for auxiliary histograms. diff --git a/sql/connection_unittest.cc b/sql/connection_unittest.cc index 10a15a8..5412be1 100644 --- a/sql/connection_unittest.cc +++ b/sql/connection_unittest.cc @@ -4,10 +4,12 @@ #include "base/bind.h" #include "base/files/file_util.h" +#include "base/files/memory_mapped_file.h" #include "base/files/scoped_file.h" #include "base/files/scoped_temp_dir.h" #include "base/logging.h" #include "base/metrics/statistics_recorder.h" +#include "base/strings/stringprintf.h" #include "base/test/histogram_tester.h" #include "sql/connection.h" #include "sql/correct_sql_test_base.h" @@ -1298,4 +1300,80 @@ TEST_F(SQLConnectionTest, TimeUpdateTransaction) { EXPECT_EQ(0, samples->sum()); } +// Make sure that OS file writes to a mmap'ed file are reflected in the memory +// mapping of a memory-mapped file. Normally SQLite writes to memory-mapped +// files using memcpy(), which should stay consistent. Our SQLite is slightly +// patched to mmap read only, then write using OS file writes. If the +// memory-mapped version doesn't reflect the OS file writes, SQLite's +// memory-mapped I/O should be disabled on this platform. +TEST_F(SQLConnectionTest, MmapTest) { + // Skip the test for platforms which don't enable memory-mapped I/O in SQLite, + // or which don't even support the pragma. The former seems to apply to iOS, + // the latter to older iOS. + // TODO(shess): Disable test on iOS? Disable on USE_SYSTEM_SQLITE? + { + sql::Statement s(db().GetUniqueStatement("PRAGMA mmap_size")); + if (!s.Step() || !s.ColumnInt64(0)) + return; + } + + // The test re-uses the database file to make sure it's representative of a + // SQLite file, but will be storing incompatible data. + db().Close(); + + const uint32 kFlags = + base::File::FLAG_OPEN|base::File::FLAG_READ|base::File::FLAG_WRITE; + char buf[4096]; + + // Create a file with a block of '0', a block of '1', and a block of '2'. + { + base::File f(db_path(), kFlags); + ASSERT_TRUE(f.IsValid()); + memset(buf, '0', sizeof(buf)); + ASSERT_EQ(f.Write(0*sizeof(buf), buf, sizeof(buf)), (int)sizeof(buf)); + + memset(buf, '1', sizeof(buf)); + ASSERT_EQ(f.Write(1*sizeof(buf), buf, sizeof(buf)), (int)sizeof(buf)); + + memset(buf, '2', sizeof(buf)); + ASSERT_EQ(f.Write(2*sizeof(buf), buf, sizeof(buf)), (int)sizeof(buf)); + } + + // mmap the file and verify that everything looks right. + { + base::MemoryMappedFile m; + ASSERT_TRUE(m.Initialize(db_path())); + + memset(buf, '0', sizeof(buf)); + ASSERT_EQ(0, memcmp(buf, m.data() + 0*sizeof(buf), sizeof(buf))); + + memset(buf, '1', sizeof(buf)); + ASSERT_EQ(0, memcmp(buf, m.data() + 1*sizeof(buf), sizeof(buf))); + + memset(buf, '2', sizeof(buf)); + ASSERT_EQ(0, memcmp(buf, m.data() + 2*sizeof(buf), sizeof(buf))); + + // Scribble some '3' into the first page of the file, and verify that it + // looks the same in the memory mapping. + { + base::File f(db_path(), kFlags); + ASSERT_TRUE(f.IsValid()); + memset(buf, '3', sizeof(buf)); + ASSERT_EQ(f.Write(0*sizeof(buf), buf, sizeof(buf)), (int)sizeof(buf)); + } + ASSERT_EQ(0, memcmp(buf, m.data() + 0*sizeof(buf), sizeof(buf))); + + // Repeat with a single '4' in case page-sized blocks are different. + const size_t kOffset = 1*sizeof(buf) + 123; + ASSERT_NE('4', m.data()[kOffset]); + { + base::File f(db_path(), kFlags); + ASSERT_TRUE(f.IsValid()); + buf[0] = '4'; + ASSERT_EQ(f.Write(kOffset, buf, 1), 1); + } + ASSERT_EQ('4', m.data()[kOffset]); + } +} + } // namespace diff --git a/sql/statement.cc b/sql/statement.cc index 09c95da..2a0faa7 100644 --- a/sql/statement.cc +++ b/sql/statement.cc @@ -110,6 +110,11 @@ void Statement::Reset(bool clear_bound_vars) { ref_->connection()->RecordOneEvent(Connection::EVENT_STATEMENT_SUCCESS); } + // Potentially release dirty cache pages if an autocommit statement made + // changes. + if (ref_->connection()) + ref_->connection()->ReleaseCacheMemoryIfNeeded(false); + succeeded_ = false; stepped_ = false; } |