Modified: trunk/Source/WebKit/ChangeLog (278777 => 278778)
--- trunk/Source/WebKit/ChangeLog 2021-06-11 20:25:41 UTC (rev 278777)
+++ trunk/Source/WebKit/ChangeLog 2021-06-11 20:34:28 UTC (rev 278778)
@@ -1,5 +1,30 @@
2021-06-11 Chris Dumez <[email protected]>
+ Regression(r276653) We're going to disk more often for local storage operations
+ https://bugs.webkit.org/show_bug.cgi?id=226832
+
+ Reviewed by Darin Adler.
+
+ We're going to disk more often for local storage operations since r276653 because we no
+ longer keep items in memory. This results in a slightly increased power usage on one of
+ our benchmarks. As a first step to improve this, I am reintroducing a cache of the items
+ in memory, as long as the values are not too large (1Kb limit). We still go to disk to
+ look up values that are larger than 1Kb to avoid regressing memory usage.
+
+ * NetworkProcess/WebStorage/LocalStorageDatabase.cpp:
+ (WebKit::LocalStorageDatabase::openDatabase):
+ (WebKit::LocalStorageDatabase::items const):
+ (WebKit::LocalStorageDatabase::removeItem):
+ (WebKit::LocalStorageDatabase::item const):
+ (WebKit::LocalStorageDatabase::itemBypassingCache const):
+ (WebKit::LocalStorageDatabase::setItem):
+ (WebKit::LocalStorageDatabase::clear):
+ (WebKit::LocalStorageDatabase::close):
+ (WebKit::LocalStorageDatabase::databaseIsEmpty const):
+ * NetworkProcess/WebStorage/LocalStorageDatabase.h:
+
+2021-06-11 Chris Dumez <[email protected]>
+
Enable WebProcess' release logging in ephemeral sessions
https://bugs.webkit.org/show_bug.cgi?id=226927
Modified: trunk/Source/WebKit/NetworkProcess/WebStorage/LocalStorageDatabase.cpp (278777 => 278778)
--- trunk/Source/WebKit/NetworkProcess/WebStorage/LocalStorageDatabase.cpp 2021-06-11 20:25:41 UTC (rev 278777)
+++ trunk/Source/WebKit/NetworkProcess/WebStorage/LocalStorageDatabase.cpp 2021-06-11 20:34:28 UTC (rev 278778)
@@ -38,7 +38,8 @@
namespace WebKit {
using namespace WebCore;
-static const ASCIILiteral getItemsQueryString { "SELECT key, value FROM ItemTable"_s };
+constexpr auto getItemsQueryString { "SELECT key, value FROM ItemTable"_s };
+constexpr unsigned maximumSizeForValuesKeptInMemory { 1024 }; // 1 KB
Ref<LocalStorageDatabase> LocalStorageDatabase::create(String&& databasePath, unsigned quotaInBytes)
{
@@ -61,8 +62,11 @@
bool LocalStorageDatabase::openDatabase(ShouldCreateDatabase shouldCreateDatabase)
{
ASSERT(!RunLoop::isMain());
- if (!FileSystem::fileExists(m_databasePath) && shouldCreateDatabase == ShouldCreateDatabase::No)
- return true;
+ if (!FileSystem::fileExists(m_databasePath)) {
+ if (shouldCreateDatabase == ShouldCreateDatabase::No)
+ return true;
+ m_items = HashMap<String, String> { };
+ }
if (m_databasePath.isEmpty()) {
LOG_ERROR("Filename for local storage database is empty - cannot open for persistent storage");
@@ -140,6 +144,16 @@
if (!m_database.isOpen())
return { };
+ HashMap<String, String> items;
+ if (m_items) {
+ items.reserveInitialCapacity(m_items->size());
+ for (auto& entry : *m_items) {
+ auto value = entry.value.isNull() ? itemBypassingCache(entry.key) : entry.value;
+ items.add(entry.key, WTFMove(value));
+ }
+ return items;
+ }
+
auto query = scopedStatement(m_getItemsStatement, getItemsQueryString);
if (!query) {
LOG_ERROR("Unable to select items from ItemTable for local storage");
@@ -146,13 +160,15 @@
return { };
}
- HashMap<String, String> items;
+ m_items = HashMap<String, String> { };
int result = query->step();
while (result == SQLITE_ROW) {
String key = query->columnText(0);
String value = query->columnBlobAsString(1);
- if (!key.isNull() && !value.isNull())
+ if (!key.isNull() && !value.isNull()) {
+ m_items->add(key, value.sizeInBytes() > maximumSizeForValuesKeptInMemory ? String() : value);
items.add(WTFMove(key), WTFMove(value));
+ }
result = query->step();
}
@@ -184,6 +200,9 @@
LOG_ERROR("Failed to delete item in the local storage database - %i", result);
return;
}
+
+ if (m_items)
+ m_items->remove(key);
}
String LocalStorageDatabase::item(const String& key) const
@@ -192,6 +211,20 @@
if (!m_database.isOpen())
return { };
+ if (m_items) {
+ // Use find() instead of get() since a null String is a valid value here.
+ auto it = m_items->find(key);
+ if (it == m_items->end())
+ return { };
+ if (!it->value.isNull())
+ return it->value;
+ // The value is too large and needs to be fetched from the database.
+ }
+ return itemBypassingCache(key);
+}
+
+String LocalStorageDatabase::itemBypassingCache(const String& key) const
+{
auto query = scopedStatement(m_getItemStatement, "SELECT value FROM ItemTable WHERE key=?"_s);
if (!query) {
LOG_ERROR("Unable to get item from ItemTable for local storage");
@@ -231,7 +264,11 @@
LOG_ERROR("Failed to update item in the local storage database - %i", result);
if (result == SQLITE_FULL)
quotaException = true;
+ return;
}
+
+ if (m_items)
+ m_items->set(key, value.sizeInBytes() > maximumSizeForValuesKeptInMemory ? String() : value);
}
bool LocalStorageDatabase::clear()
@@ -240,6 +277,9 @@
if (!m_database.isOpen())
return false;
+ if (m_items && m_items->isEmpty())
+ return false;
+
auto clearStatement = scopedStatement(m_clearStatement, "DELETE FROM ItemTable"_s);
if (!clearStatement) {
LOG_ERROR("Failed to prepare clear statement - cannot write to local storage database");
@@ -252,6 +292,11 @@
return false;
}
+ if (m_items) {
+ m_items->clear();
+ return true;
+ }
+
return m_database.lastChanges() > 0;
}
@@ -269,6 +314,7 @@
m_getItemStatement = nullptr;
m_getItemsStatement = nullptr;
m_deleteItemStatement = nullptr;
+ m_items = std::nullopt;
if (m_database.isOpen())
m_database.close();
@@ -283,6 +329,9 @@
if (!m_database.isOpen())
return false;
+ if (m_items)
+ return m_items->isEmpty();
+
auto query = m_database.prepareStatement("SELECT COUNT(*) FROM ItemTable"_s);
if (!query) {
LOG_ERROR("Unable to count number of rows in ItemTable for local storage");
Modified: trunk/Source/WebKit/NetworkProcess/WebStorage/LocalStorageDatabase.h (278777 => 278778)
--- trunk/Source/WebKit/NetworkProcess/WebStorage/LocalStorageDatabase.h 2021-06-11 20:25:41 UTC (rev 278777)
+++ trunk/Source/WebKit/NetworkProcess/WebStorage/LocalStorageDatabase.h 2021-06-11 20:34:28 UTC (rev 278778)
@@ -62,6 +62,8 @@
bool migrateItemTableIfNeeded();
bool databaseIsEmpty() const;
+ String itemBypassingCache(const String& key) const;
+
WebCore::SQLiteStatementAutoResetScope scopedStatement(std::unique_ptr<WebCore::SQLiteStatement>&, ASCIILiteral query) const;
String m_databasePath;
@@ -69,6 +71,10 @@
const unsigned m_quotaInBytes { 0 };
bool m_isClosed { false };
+ // Cached version of the items in memory.
+ // If the value is too large to keep in memory, we store a null String.
+ mutable std::optional<HashMap<String, String>> m_items;
+
mutable std::unique_ptr<WebCore::SQLiteStatement> m_clearStatement;
mutable std::unique_ptr<WebCore::SQLiteStatement> m_insertStatement;
mutable std::unique_ptr<WebCore::SQLiteStatement> m_getItemStatement;
