Allow controlling mmap behavior through DatabaseOptions.
This also gets rid of the ability to disable mmap process-wide since it's not needed and brittle global state. Bug: 40144971 Change-Id: Id7dbb1537a88c7903bf9e2b68a70ba1e9d5b19c6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6239956 Reviewed-by: Etienne Bergeron <etienneb@chromium.org> Reviewed-by: Anthony Vallée-Dubois <anthonyvd@chromium.org> Reviewed-by: Tsuyoshi Horo <horo@chromium.org> Commit-Queue: Olivier Li <olivierli@chromium.org> Cr-Commit-Position: refs/heads/main@{#1419443}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
7ea460d20b
commit
3b4ef073f6
components/services/storage
sql
@ -212,6 +212,9 @@ SharedStorageDatabase::SharedStorageDatabase(
|
||||
: db_(sql::DatabaseOptions()
|
||||
.set_wal_mode(base::FeatureList::IsEnabled(
|
||||
blink::features::kSharedStorageAPIEnableWALForDatabase))
|
||||
// Prevent SQLite from trying to use mmap, as SandboxedVfs does
|
||||
// not currently support this.
|
||||
.set_mmap_enabled(false)
|
||||
// We DCHECK that the page size is valid in the constructor for
|
||||
// `SharedStorageOptions`.
|
||||
.set_page_size(options->max_page_size)
|
||||
|
@ -78,12 +78,6 @@ void StorageServiceImpl::SetDataDirectory(
|
||||
weak_ptr_factory_.GetWeakPtr()),
|
||||
base::SequencedTaskRunner::GetCurrentDefault()));
|
||||
|
||||
// Prevent SQLite from trying to use mmap, as SandboxedVfs does not currently
|
||||
// support this.
|
||||
//
|
||||
// TODO(crbug.com/40144971): Configure this per Database instance.
|
||||
sql::Database::DisableMmapByDefault();
|
||||
|
||||
// SQLite needs our VFS implementation to work over a FilesystemProxy. This
|
||||
// installs it as the default implementation for the service process.
|
||||
sql::SandboxedVfs::Register(
|
||||
|
@ -77,8 +77,6 @@ namespace sql {
|
||||
|
||||
namespace {
|
||||
|
||||
bool enable_mmap_by_default_ = true;
|
||||
|
||||
// The name of the main database associated with a sqlite3* connection.
|
||||
//
|
||||
// SQLite has the ability to ATTACH multiple databases to the same connection.
|
||||
@ -355,7 +353,7 @@ Database::Database(Database::Tag tag) : Database(DatabaseOptions{}, tag) {}
|
||||
|
||||
Database::Database(DatabaseOptions options, Database::Tag tag)
|
||||
: options_(options),
|
||||
mmap_disabled_(!enable_mmap_by_default_),
|
||||
mmap_disabled_(!options.mmap_enabled_),
|
||||
histogram_tag_(tag.value),
|
||||
tracing_track_name_(base::StrCat({"Database: ", histogram_tag_})) {
|
||||
DCHECK_GE(options.page_size_, 512);
|
||||
@ -375,11 +373,6 @@ Database::~Database() {
|
||||
Close();
|
||||
}
|
||||
|
||||
// static
|
||||
void Database::DisableMmapByDefault() {
|
||||
enable_mmap_by_default_ = false;
|
||||
}
|
||||
|
||||
bool Database::Open(const base::FilePath& path) {
|
||||
std::string path_string = AsUTF8ForSQL(path);
|
||||
TRACE_EVENT1("sql", "Database::Open", "path", path_string);
|
||||
|
@ -248,6 +248,14 @@ struct COMPONENT_EXPORT(SQL) DatabaseOptions {
|
||||
return *this;
|
||||
}
|
||||
|
||||
// If true database attempts using memory mapped files. True by default. Only
|
||||
// set to false when a condition is known that prevents the use of memory
|
||||
// mapped files. See https://www.sqlite.org/mmap.html.
|
||||
DatabaseOptions& set_mmap_enabled(bool mmap_enabled) {
|
||||
mmap_enabled_ = mmap_enabled;
|
||||
return *this;
|
||||
}
|
||||
|
||||
private:
|
||||
friend class Database;
|
||||
FRIEND_TEST_ALL_PREFIXES(DatabaseOptionsTest,
|
||||
@ -266,6 +274,7 @@ struct COMPONENT_EXPORT(SQL) DatabaseOptions {
|
||||
bool mmap_alt_status_discouraged_ = false;
|
||||
bool enable_views_discouraged_ = false;
|
||||
const char* vfs_name_discouraged_ = nullptr;
|
||||
bool mmap_enabled_ = true;
|
||||
};
|
||||
|
||||
// Holds database diagnostics in a structured format.
|
||||
@ -382,12 +391,6 @@ class COMPONENT_EXPORT(SQL) Database {
|
||||
Database& operator=(Database&&) = delete;
|
||||
~Database();
|
||||
|
||||
// Allows mmapping to be disabled globally by default in the calling process.
|
||||
// Must be called before any threads attempt to create a Database.
|
||||
//
|
||||
// TODO(crbug.com/40144971): Remove this global configuration.
|
||||
static void DisableMmapByDefault();
|
||||
|
||||
// Pre-init configuration ----------------------------------------------------
|
||||
|
||||
// The page size that will be used when creating a new database.
|
||||
|
Reference in New Issue
Block a user