sql: Feature flag for in-memory temporary storage.
This CL introduces SqlTempStoreMemory flag, which is intended to be
managed via Finch. When set, the flag causes sql::Database to run a
PRAGMA temp_store=MEMORY [1] query on every opened database.
This approach is intended to approximate the effect of building SQLite
with the SQLITE_TEMP_STORE=3 [2] macro, to measure the memory
consumption impact. Ideally, we'd test the macro directly, but //sql is
a core component of Chrome, so we can't load different versions of it
based on Finch.
[1] https://www.sqlite.org/pragma.html#pragma_temp_store
[2] https://www.sqlite.org/compile.html#temp_store
Bug: 875538
Change-Id: I537d90d763be1100503ed4bd2ada2ee19eb090bb
Reviewed-on: https://chromium-review.googlesource.com/1180530
Reviewed-by: Mark Pearson <[email protected]>
Reviewed-by: Chris Mumford <[email protected]>
Commit-Queue: Victor Costan <[email protected]>
Cr-Commit-Position: refs/heads/master@{#584652}diff --git a/sql/BUILD.gn b/sql/BUILD.gn
index 98c6265bf..0a220ec 100644
--- a/sql/BUILD.gn
+++ b/sql/BUILD.gn
@@ -21,6 +21,8 @@
"recovery.cc",
"recovery.h",
"sql_export.h",
+ "sql_features.cc",
+ "sql_features.h",
"sql_memory_dump_provider.cc",
"sql_memory_dump_provider.h",
"statement.cc",
diff --git a/sql/database.cc b/sql/database.cc
index 66bf05b..ffe7c6b 100644
--- a/sql/database.cc
+++ b/sql/database.cc
@@ -33,6 +33,7 @@
#include "sql/database_memory_dump_provider.h"
#include "sql/initialization.h"
#include "sql/meta_table.h"
+#include "sql/sql_features.h"
#include "sql/statement.h"
#include "sql/vfs_wrapper.h"
#include "third_party/sqlite/sqlite3.h"
@@ -1674,6 +1675,12 @@
ignore_result(Execute("PRAGMA locking_mode=EXCLUSIVE"));
}
+ if (base::FeatureList::IsEnabled(features::kSqlTempStoreMemory)) {
+ err = ExecuteAndReturnErrorCode("PRAGMA temp_store=MEMORY");
+ // This operates on in-memory configuration, so it should not fail.
+ DCHECK_EQ(err, SQLITE_OK) << "Failed switching to in-RAM temporary storage";
+ }
+
// http://www.sqlite.org/pragma.html#pragma_journal_mode
// DELETE (default) - delete -journal file to commit.
// TRUNCATE - truncate -journal file to commit.
@@ -1681,7 +1688,7 @@
// TRUNCATE should be faster than DELETE because it won't need directory
// changes for each transaction. PERSIST may break the spirit of using
// secure_delete.
- ignore_result(Execute("PRAGMA journal_mode = TRUNCATE"));
+ ignore_result(Execute("PRAGMA journal_mode=TRUNCATE"));
const base::TimeDelta kBusyTimeout =
base::TimeDelta::FromSeconds(kBusyTimeoutSeconds);
@@ -1728,7 +1735,7 @@
// 64-bit platforms.
size_t mmap_size = mmap_disabled_ ? 0 : GetAppropriateMmapSize();
std::string mmap_sql =
- base::StringPrintf("PRAGMA mmap_size = %" PRIuS, mmap_size);
+ base::StringPrintf("PRAGMA mmap_size=%" PRIuS, mmap_size);
ignore_result(Execute(mmap_sql.c_str()));
// Determine if memory-mapping has actually been enabled. The Execute() above
@@ -1889,7 +1896,7 @@
// allows SQLite to process through certain cases of corruption.
// Failing to set this pragma probably means that the database is
// beyond recovery.
- static const char kWritableSchemaSql[] = "PRAGMA writable_schema = ON";
+ static const char kWritableSchemaSql[] = "PRAGMA writable_schema=ON";
if (!Execute(kWritableSchemaSql))
return false;
@@ -1909,7 +1916,7 @@
}
// Best effort to put things back as they were before.
- static const char kNoWritableSchemaSql[] = "PRAGMA writable_schema = OFF";
+ static const char kNoWritableSchemaSql[] = "PRAGMA writable_schema=OFF";
ignore_result(Execute(kNoWritableSchemaSql));
return ret;
diff --git a/sql/database_unittest.cc b/sql/database_unittest.cc
index 57a4505..78c5bfb 100644
--- a/sql/database_unittest.cc
+++ b/sql/database_unittest.cc
@@ -13,12 +13,14 @@
#include "base/macros.h"
#include "base/strings/string_number_conversions.h"
#include "base/test/metrics/histogram_tester.h"
+#include "base/test/scoped_feature_list.h"
#include "base/test/simple_test_tick_clock.h"
#include "base/trace_event/process_memory_dump.h"
#include "build/build_config.h"
#include "sql/database.h"
#include "sql/database_memory_dump_provider.h"
#include "sql/meta_table.h"
+#include "sql/sql_features.h"
#include "sql/statement.h"
#include "sql/test/error_callback_support.h"
#include "sql/test/scoped_error_expecter.h"
@@ -100,7 +102,6 @@
namespace {
-using sql::test::ExecuteWithResults;
using sql::test::ExecuteWithResult;
// Helper to return the count of items in sqlite_master. Return -1 in
@@ -1618,4 +1619,22 @@
#endif // !defined(OS_ANDROID) && !defined(OS_IOS) && !defined(OS_FUCHSIA)
}
+// Verify that Raze() can handle an empty file. SQLite should treat
+// this as an empty database.
+TEST_F(SQLDatabaseTest, SqlTempMemoryFeatureFlagDefault) {
+ EXPECT_EQ("0", ExecuteWithResult(&db(), "PRAGMA temp_store"))
+ << "temp_store should not be set by default";
+}
+
+TEST_F(SQLDatabaseTest, SqlTempMemoryFeatureFlagEnabled) {
+ base::test::ScopedFeatureList feature_list;
+ feature_list.InitAndEnableFeature(features::kSqlTempStoreMemory);
+
+ db().Close();
+
+ ASSERT_TRUE(db().Open(db_path()));
+ EXPECT_EQ("2", ExecuteWithResult(&db(), "PRAGMA temp_store"))
+ << "temp_store should be set by the feature flag SqlTempStoreMemory";
+}
+
} // namespace sql
diff --git a/sql/sql_features.cc b/sql/sql_features.cc
new file mode 100644
index 0000000..c7cf33e
--- /dev/null
+++ b/sql/sql_features.cc
@@ -0,0 +1,23 @@
+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "sql/sql_features.h"
+
+namespace sql {
+
+namespace features {
+
+// SQLite databases only use RAM for temporary storage.
+//
+// Enabling this feature matches the SQLITE_TEMP_STORE=3 build option, which is
+// used on Android.
+//
+// TODO(pwnall): After the memory impact of the config change is assessed, land
+// https://crrev.com/c/1146493 and remove this flag.
+const base::Feature kSqlTempStoreMemory{"SqlTempStoreMemory",
+ base::FEATURE_DISABLED_BY_DEFAULT};
+
+} // namespace features
+
+} // namespace sql
diff --git a/sql/sql_features.h b/sql/sql_features.h
new file mode 100644
index 0000000..4b0c947
--- /dev/null
+++ b/sql/sql_features.h
@@ -0,0 +1,21 @@
+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef SQL_SQL_FEATURES_H_
+#define SQL_SQL_FEATURES_H_
+
+#include "base/feature_list.h"
+#include "sql/sql_export.h"
+
+namespace sql {
+
+namespace features {
+
+SQL_EXPORT extern const base::Feature kSqlTempStoreMemory;
+
+} // namespace features
+
+} // namespace sql
+
+#endif // SQL_SQL_FEATURES_H_
