Commit c6d3a866 authored by Victor Costan's avatar Victor Costan Committed by Commit Bot

sqlite: Use SQLite's API for getting per-database page pools.

Chrome currently carries a SQLite patch at
0002-Use-seperate-page-cache-pools-for-each-sqlite-connec.patch
which forces per-database (technically, per-connection) page cache
pools. Asides from adding to our maintenance burden, the patch breaks an
assert() in SQLite's code, suggesting that it doesn't work as intended
anymore.

Fortunately, SQLite has a few supported ways of specifying whether
database connections use private page cache pools or a central pool. The
most convenient method for Chrome is the SQLITE_OPEN_PRIVATECACHE flag
on sqlite3_open_v2() [1].

This CL does the following:

1) Removes the SQLITE_SEPARATE_CACHE_POOLS build flag, which activates
   Chrome's page cache pool patch. The patch (which becomes inert when
   this CL lands) shall be removed from our SQLite version in a
   follow-up CL, after this CL sticks.
2) Removes the SQLITE_ENABLE_MEMORY_MANAGEMENT build flag. This flag is
   incompatible with private page cache pools, because it enables the
   sqlite3_release_memory() function [2], which assumes a centralized
   (shared) page cache pool [3]. Chrome doesn't use
   sqlite3_release_memory() today, so the flag is unnecessary. If we
   want to trim SQLite caches in response to memory pressure, we can
   still use sqlite3_db_release_memory() [4] and track all the open
   connections ourselves.
3) Adds the SQLITE_OPEN_PRIVATECACHE flag to our invocation of
   sqlite3_open_v2().

[1] https://www.sqlite.org/c3ref/open.html
[2] https://www.sqlite.org/c3ref/release_memory.html
[3] https://www.sqlite.org/sharedcache.html
[4] https://www.sqlite.org/c3ref/db_release_memory.html

Bug: 906396
Change-Id: Ib3b08b26364ad86d173111fc4e554bd07bd91aa1
Reviewed-on: https://chromium-review.googlesource.com/c/1343789Reviewed-by: default avatarJoshua Bell <jsbell@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609754}
parent edbaaa23
...@@ -1612,9 +1612,17 @@ bool Database::OpenInternal(const std::string& file_name, ...@@ -1612,9 +1612,17 @@ bool Database::OpenInternal(const std::string& file_name,
// Custom memory-mapping VFS which reads pages using regular I/O on first hit. // Custom memory-mapping VFS which reads pages using regular I/O on first hit.
sqlite3_vfs* vfs = VFSWrapper(); sqlite3_vfs* vfs = VFSWrapper();
const char* vfs_name = (vfs ? vfs->zName : nullptr); const char* vfs_name = (vfs ? vfs->zName : nullptr);
int err =
sqlite3_open_v2(file_name.c_str(), &db_, // The flags are documented at https://www.sqlite.org/c3ref/open.html.
SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE, vfs_name); //
// Chrome uses SQLITE_OPEN_PRIVATECACHE because SQLite is used by many
// disparate features with their own databases, and having separate page
// caches makes it easier to reason about each feature's performance in
// isolation.
int err = sqlite3_open_v2(
file_name.c_str(), &db_,
SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_PRIVATECACHE,
vfs_name);
if (err != SQLITE_OK) { if (err != SQLITE_OK) {
// Extended error codes cannot be enabled until a handle is // Extended error codes cannot be enabled until a handle is
// available, fetch manually. // available, fetch manually.
......
...@@ -62,9 +62,10 @@ int SQLiteFileSystem::OpenDatabase(const String& filename, sqlite3** database) { ...@@ -62,9 +62,10 @@ int SQLiteFileSystem::OpenDatabase(const String& filename, sqlite3** database) {
<< "InitializeSQLite() must be called before " << __func__; << "InitializeSQLite() must be called before " << __func__;
#endif // DCHECK_IS_ON() #endif // DCHECK_IS_ON()
return sqlite3_open_v2(filename.Utf8().data(), database, return sqlite3_open_v2(
SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE, filename.Utf8().data(), database,
"chromium_vfs"); SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_PRIVATECACHE,
"chromium_vfs");
} }
} // namespace blink } // namespace blink
...@@ -28,22 +28,12 @@ config("chromium_sqlite3_compile_options") { ...@@ -28,22 +28,12 @@ config("chromium_sqlite3_compile_options") {
"SQLITE_DISABLE_FTS4_DEFERRED", "SQLITE_DISABLE_FTS4_DEFERRED",
"SQLITE_ENABLE_ICU", "SQLITE_ENABLE_ICU",
# Enables memory tracking needed to release unused memory.
#
# Needed for sqlite3_release_memory() and its variants to work. Without this
# option, the interfaces exist, but the methods are no-ops.
"SQLITE_ENABLE_MEMORY_MANAGEMENT",
# Defaults the secure_delete pragma to 1. # Defaults the secure_delete pragma to 1.
# #
# This causes SQLite to overwrite all deleted information with zeroes, # This causes SQLite to overwrite all deleted information with zeroes,
# trading additional I/O for better privacy guarantees. # trading additional I/O for better privacy guarantees.
"SQLITE_SECURE_DELETE", "SQLITE_SECURE_DELETE",
# Custom flag to tweak pcache pools.
# TODO(pwnall): This shouldn't use faux-SQLite naming.
"SQLITE_SEPARATE_CACHE_POOLS",
# TODO(pwnall): SQLite adds mutexes to protect structures which cross # TODO(pwnall): SQLite adds mutexes to protect structures which cross
# threads. In theory Chrome should be able to turn this to "2" which # threads. In theory Chrome should be able to turn this to "2" which
# should give a slight speed boost. "2" is safe as long as a single # should give a slight speed boost. "2" is safe as long as a single
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment