corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 1964400 - IDB: Replace array with linked list in DatabaseActorInfo to speed up live database handling; r=dom-storage-reviewers,jstutte

Browse Source 
Replaces the mLiveDatabases array in DatabaseActorInfo with a linked list to
improve performance during live database tracking.

This change improves performance in scenarios where many databases are closed
in quick succession. The array previously used for tracking live Database
instances incurred linear-time removal overhead, which the linked list avoids.
Unlike a hash set, the linked list preserves insertion order, which is required
for correctness.

This is part of the ongoing effort to reduce jank caused by frequent IndexedDB
open/close activity, tracked in the associated meta bug.

Differential Revision: https://phabricator.services.mozilla.com/D247884
This commit is contained in:
Jan Varga
2025-05-07 16:24:51 +00:00
committed by jvarga@mozilla.com
parent 8f9b0f7118
commit 3e35b9d0b7
1 changed files with 52 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files

98
dom/indexedDB/ActorsParent.cpp
View File

@@ -2125,10 +2125,9 @@ class WaitForTransactionsHelper final : public Runnable {
NS_DECL_NSIRUNNABLE
};
class Database final
: public PBackgroundIDBDatabaseParent,
public SupportsCheckedUnsafePtr<CheckIf<DiagnosticAssertEnabled>>,
public AtomicSafeRefCounted<Database> {
class Database final : public PBackgroundIDBDatabaseParent,
public LinkedListElement<Database>,
public AtomicSafeRefCounted<Database> {
friend class VersionChangeTransaction;
class StartTransactionOp;
@@ -2318,6 +2317,7 @@ class Database final
~Database() override {
MOZ_ASSERT(mClosed);
MOZ_ASSERT_IF(mActorWasAlive, mActorDestroyed);
MOZ_DIAGNOSTIC_ASSERT(!isInList());
NS_ProxyRelease("ReleaseIDBFactory", mBackgroundThread.get(),
mFactory.forget());
@@ -4722,7 +4722,13 @@ struct DatabaseActorInfo final {
friend class mozilla::DefaultDelete<DatabaseActorInfo>;
SafeRefPtr<FullDatabaseMetadata> mMetadata;
nsTArray<NotNull<CheckedUnsafePtr<Database>>> mLiveDatabases;
// We don't use LinkedList<CheckedUnsafePtr<Database>> because
// CheckedUnsafePtr is not suitable for use within LinkedList. While it's
// theoretically possible to adapt LinkedList to support it, doing so would
// introduce unnecessary overhead. Instead, we use a simpler and more
// efficient approach. Each Database instance asserts !isInList() in its
// destructor to catch dangling pointer issues.
LinkedList<Database> mLiveDatabases;
RefPtr<FactoryOp> mWaitingFactoryOp;
DatabaseActorInfo(SafeRefPtr<FullDatabaseMetadata> aMetadata,
@@ -4730,12 +4736,12 @@ struct DatabaseActorInfo final {
: mMetadata(std::move(aMetadata)) {
MOZ_COUNT_CTOR(DatabaseActorInfo);
mLiveDatabases.AppendElement(aDatabase);
mLiveDatabases.insertBack(aDatabase);
}
private:
~DatabaseActorInfo() {
MOZ_ASSERT(mLiveDatabases.IsEmpty());
MOZ_ASSERT(mLiveDatabases.isEmpty());
MOZ_ASSERT(!mWaitingFactoryOp || !mWaitingFactoryOp->HasBlockedDatabases());
MOZ_COUNT_DTOR(DatabaseActorInfo);
@@ -6022,10 +6028,10 @@ void InvalidateLiveDatabasesMatching(const Condition& aCondition) {
nsTArray<SafeRefPtr<Database>> databases;
for (const auto& liveDatabasesEntry : gLiveDatabaseHashtable->Values()) {
for (const auto& database : liveDatabasesEntry->mLiveDatabases) {
for (Database* const database : liveDatabasesEntry->mLiveDatabases) {
if (aCondition(*database)) {
databases.AppendElement(
SafeRefPtr{database.get(), AcquireStrongRefFromRawPtr{}});
SafeRefPtr{database, AcquireStrongRefFromRawPtr{}});
}
}
}
@@ -9498,7 +9504,7 @@ bool Database::CloseInternal() {
DatabaseActorInfo* info;
MOZ_ALWAYS_TRUE(gLiveDatabaseHashtable->Get(Id(), &info));
MOZ_ASSERT(info->mLiveDatabases.Contains(this));
MOZ_ASSERT(info->mLiveDatabases.contains(this));
if (info->mWaitingFactoryOp) {
info->mWaitingFactoryOp->NoteDatabaseClosed(this);
@@ -9550,12 +9556,12 @@ void Database::CleanupMetadata() {
DatabaseActorInfo* info;
MOZ_ALWAYS_TRUE(gLiveDatabaseHashtable->Get(Id(), &info));
MOZ_ALWAYS_TRUE(info->mLiveDatabases.RemoveElement(this));
removeFrom(info->mLiveDatabases);
QuotaManager::MaybeRecordQuotaClientShutdownStep(
quota::Client::IDB, "Live database entry removed"_ns);
if (info->mLiveDatabases.IsEmpty()) {
if (info->mLiveDatabases.isEmpty()) {
MOZ_ASSERT(!info->mWaitingFactoryOp ||
!info->mWaitingFactoryOp->HasBlockedDatabases());
gLiveDatabaseHashtable->Remove(Id());
@@ -9757,7 +9763,7 @@ mozilla::ipc::IPCResult Database::RecvBlocked() {
DatabaseActorInfo* info;
MOZ_ALWAYS_TRUE(gLiveDatabaseHashtable->Get(Id(), &info));
MOZ_ASSERT(info->mLiveDatabases.Contains(this));
MOZ_ASSERT(info->mLiveDatabases.contains(this));
if (NS_WARN_IF(!info->mWaitingFactoryOp)) {
return IPC_FAIL(this, "Database info has no mWaitingFactoryOp!");
@@ -10778,14 +10784,14 @@ bool VersionChangeTransaction::CopyDatabaseMetadata() {
// Replace the live metadata with the new mutable copy.
DatabaseActorInfo* info;
MOZ_ALWAYS_TRUE(gLiveDatabaseHashtable->Get(origMetadata.mDatabaseId, &info));
MOZ_ASSERT(!info->mLiveDatabases.IsEmpty());
MOZ_ASSERT(!info->mLiveDatabases.isEmpty());
MOZ_ASSERT(info->mMetadata == &origMetadata);
mOldMetadata = std::move(info->mMetadata);
info->mMetadata = std::move(newMetadata);
// Replace metadata pointers for all live databases.
for (const auto& liveDatabase : info->mLiveDatabases) {
for (Database* const liveDatabase : info->mLiveDatabases) {
liveDatabase->mMetadata = info->mMetadata.clonePtr();
}
@@ -10809,7 +10815,7 @@ void VersionChangeTransaction::UpdateMetadata(nsresult aResult) {
return;
}
MOZ_ASSERT(!info->mLiveDatabases.IsEmpty());
MOZ_ASSERT(!info->mLiveDatabases.isEmpty());
if (NS_SUCCEEDED(aResult)) {
// Remove all deleted objectStores and indexes, then mark immutable.
@@ -10840,7 +10846,7 @@ void VersionChangeTransaction::UpdateMetadata(nsresult aResult) {
// Replace metadata pointers for all live databases.
info->mMetadata = std::move(oldMetadata);
for (auto& liveDatabase : info->mLiveDatabases) {
for (Database* const liveDatabase : info->mLiveDatabases) {
liveDatabase->mMetadata = info->mMetadata.clonePtr();
}
}
@@ -12621,6 +12627,7 @@ nsCString QuotaClient::GetShutdownStatus() const {
// XXX It might be confusing to remove duplicates here, as the actual list
// won't match the count then.
nsTHashSet<nsCString> ids;
std::transform(gFactoryOps->cbegin(), gFactoryOps->cend(),
MakeInserter(ids), [](const auto& factoryOp) {
MOZ_ASSERT(factoryOp);
@@ -12639,16 +12646,15 @@ nsCString QuotaClient::GetShutdownStatus() const {
data.Append("LiveDatabases: "_ns +
IntToCString(gLiveDatabaseHashtable->Count()) + " ("_ns);
// XXX What's the purpose of adding these to a hashtable before joining them
// to the string? (Maybe this used to be an ordered container before???)
// XXX It might be confusing to remove duplicates here, as the actual list
// won't match the count then.
nsTHashSet<nsCString> ids;
for (const auto& entry : gLiveDatabaseHashtable->Values()) {
MOZ_ASSERT(entry);
std::transform(entry->mLiveDatabases.cbegin(),
entry->mLiveDatabases.cend(), MakeInserter(ids),
[](const auto& database) {
std::transform(entry->mLiveDatabases.begin(), entry->mLiveDatabases.end(),
MakeInserter(ids), [](const Database* const database) {
nsCString id;
database->Stringify(id);
return id;
@@ -13442,16 +13448,19 @@ nsresult Maintenance::BeginDatabaseMaintenance() {
}
if (gLiveDatabaseHashtable) {
return std::all_of(
gLiveDatabaseHashtable->Values().cbegin(),
gLiveDatabaseHashtable->Values().cend(),
[&aDatabasePath](const auto& liveDatabasesEntry) {
const auto& liveDatabases = liveDatabasesEntry->mLiveDatabases;
return std::all_of(liveDatabases.cbegin(), liveDatabases.cend(),
[&aDatabasePath](const auto& database) {
return database->FilePath() != aDatabasePath;
});
});
return std::all_of(gLiveDatabaseHashtable->Values().cbegin(),
gLiveDatabaseHashtable->Values().cend(),
[&aDatabasePath](const auto& liveDatabasesEntry) {
// XXX std::all_of currently doesn't work with
// LinkedList's iterator. See bug 1964969.
for (const Database* const database :
liveDatabasesEntry->mLiveDatabases) {
if (database->FilePath() == aDatabasePath) {
return false;
}
}
return true;
});
}
return true;
@@ -15028,15 +15037,14 @@ nsresult FactoryOp::SendVersionChangeMessages(
MOZ_ASSERT(!IsActorDestroyed());
const uint32_t expectedCount = mDeleting ? 0 : 1;
const uint32_t liveCount = aDatabaseActorInfo->mLiveDatabases.Length();
const uint32_t liveCount = aDatabaseActorInfo->mLiveDatabases.length();
if (liveCount > expectedCount) {
nsTArray<MaybeBlockedDatabaseInfo> maybeBlockedDatabases;
for (const auto& database : aDatabaseActorInfo->mLiveDatabases) {
if ((!aOpeningDatabase || database.get() != &aOpeningDatabase.ref()) &&
for (Database* const database : aDatabaseActorInfo->mLiveDatabases) {
if ((!aOpeningDatabase || database != &aOpeningDatabase.ref()) &&
!database->IsClosed() &&
NS_WARN_IF(!maybeBlockedDatabases.AppendElement(
SafeRefPtr{database.get(), AcquireStrongRefFromRawPtr{}},
fallible))) {
SafeRefPtr{database, AcquireStrongRefFromRawPtr{}}, fallible))) {
return NS_ERROR_OUT_OF_MEMORY;
}
}
@@ -15784,7 +15792,7 @@ nsresult OpenDatabaseOp::BeginVersionChange() {
DatabaseActorInfo* info;
MOZ_ALWAYS_TRUE(gLiveDatabaseHashtable->Get(mDatabaseId.ref(), &info));
MOZ_ASSERT(info->mLiveDatabases.Contains(mDatabase.unsafeGetRawPtr()));
MOZ_ASSERT(info->mLiveDatabases.contains(mDatabase.unsafeGetRawPtr()));
MOZ_ASSERT(!info->mWaitingFactoryOp);
MOZ_ASSERT(info->mMetadata == mMetadata);
@@ -16086,8 +16094,7 @@ void OpenDatabaseOp::EnsureDatabaseActor() {
std::move(mDirectoryLockHandle), mInPrivateBrowsing, maybeKey);
if (info) {
info->mLiveDatabases.AppendElement(
WrapNotNullUnchecked(mDatabase.unsafeGetRawPtr()));
info->mLiveDatabases.insertBack(mDatabase.unsafeGetRawPtr());
} else {
// XXX Maybe use LookupOrInsertWith above, to avoid a second lookup here?
info = gLiveDatabaseHashtable
@@ -16704,19 +16711,18 @@ void DeleteDatabaseOp::VersionChangeOp::RunOnOwningThread() {
// Inform all the other databases that they are now invalidated. That
// should remove the previous metadata from our table.
if (gLiveDatabaseHashtable->Get(deleteOp->mDatabaseId.ref(), &info)) {
MOZ_ASSERT(!info->mLiveDatabases.IsEmpty());
MOZ_ASSERT(!info->mLiveDatabases.isEmpty());
MOZ_ASSERT(!info->mWaitingFactoryOp);
nsTArray<SafeRefPtr<Database>> liveDatabases;
if (NS_WARN_IF(!liveDatabases.SetCapacity(info->mLiveDatabases.Length(),
if (NS_WARN_IF(!liveDatabases.SetCapacity(info->mLiveDatabases.length(),
fallible))) {
deleteOp->SetFailureCode(NS_ERROR_OUT_OF_MEMORY);
} else {
std::transform(info->mLiveDatabases.cbegin(),
info->mLiveDatabases.cend(),
std::transform(info->mLiveDatabases.begin(), info->mLiveDatabases.end(),
MakeBackInserter(liveDatabases),
[](const auto& aDatabase) -> SafeRefPtr<Database> {
return {aDatabase.get(), AcquireStrongRefFromRawPtr{}};
[](Database* const aDatabase) -> SafeRefPtr<Database> {
return {aDatabase, AcquireStrongRefFromRawPtr{}};
});
#ifdef DEBUG