Bug 1471025: Part 6 - Optimize preference lookups while dispatching callbacks. r=njn
Since lookups in the snapshotted hashtable are currently O(log n) rather than
O(1), they're expected to be slightly more expensive than the previous
purely-dynamic lookups.
In general, I expect this not to be a major issue. The main concern is pref
cache lookups while initializing the database. Initialization in the parent
process will still always use only a dynamic hashtable, so the performance
characteristics there won't change.
In the child process, though, we'll still need to notify observers of
preferences in the snapshot when they have user values, which could require
any number of lookups at startup (though in practice probably will not).
This patch solves that problem by caching the wrapper for the current known
preference value when dispatching callbacks, and optimizing lookups for that
value when it is present.
MozReview-Commit-ID: 2CAn0rM0bE9
--- a/modules/libpref/Preferences.cpp
+++ b/modules/libpref/Preferences.cpp
@@ -1466,17 +1466,23 @@ public:
Elem end() { return { *this, true }; }
};
static Pref*
pref_HashTableLookup(const char* aPrefName);
static void
-NotifyCallbacks(const char* aPrefName);
+NotifyCallbacks(const char* aPrefName, const PrefWrapper* aPref = nullptr);
+
+static void
+NotifyCallbacks(const char* aPrefName, const PrefWrapper& aPref)
+{
+ NotifyCallbacks(aPrefName, &aPref);
+}
#define PREF_HASHTABLE_INITIAL_LENGTH 1024
static PrefSaveData
pref_savePrefs()
{
MOZ_ASSERT(NS_IsMainThread());
@@ -1526,24 +1532,35 @@ pref_HashTableLookup(const char* aPrefNa
PrefEntry* entry = pref_HashTableLookupInner(aPrefName);
if (!entry) {
return nullptr;
}
return entry->mPref;
}
+// While notifying preference callbacks, this holds the wrapper for the
+// preference being notified, in order to optimize lookups.
+//
+// Note: Callbacks and lookups only happen on the main thread, so this is safe
+// to use without locking.
+static const PrefWrapper* gCallbackPref;
+
Maybe<PrefWrapper>
pref_Lookup(const char* aPrefName)
{
Maybe<PrefWrapper> result;
+ MOZ_ASSERT(NS_IsMainThread());
+
AddAccessCount(aPrefName);
- if (Pref* pref = pref_HashTableLookup(aPrefName)) {
+ if (gCallbackPref && strcmp(aPrefName, gCallbackPref->Name()) == 0) {
+ result.emplace(*gCallbackPref);
+ } else if (Pref* pref = pref_HashTableLookup(aPrefName)) {
result.emplace(pref);
}
return result;
}
static nsresult
pref_SetPref(const char* aPrefName,
@@ -1592,17 +1609,17 @@ pref_SetPref(const char* aPrefName,
return rv;
}
if (valueChanged) {
if (aKind == PrefValueKind::User && XRE_IsParentProcess()) {
Preferences::HandleDirty();
}
- NotifyCallbacks(aPrefName);
+ NotifyCallbacks(aPrefName, PrefWrapper(pref));
}
return NS_OK;
}
// Removes |node| from callback list. Returns the node after the deleted one.
static CallbackNode*
pref_RemoveCallbackNode(CallbackNode* aNode, CallbackNode* aPrevNode)
@@ -1620,20 +1637,23 @@ pref_RemoveCallbackNode(CallbackNode* aN
if (gLastPriorityNode == aNode) {
gLastPriorityNode = aPrevNode;
}
delete aNode;
return next_node;
}
static void
-NotifyCallbacks(const char* aPrefName)
+NotifyCallbacks(const char* aPrefName, const PrefWrapper* aPref)
{
bool reentered = gCallbacksInProgress;
+ gCallbackPref = aPref;
+ auto cleanup = MakeScopeExit([]() { gCallbackPref = nullptr; });
+
// Nodes must not be deleted while gCallbacksInProgress is true.
// Nodes that need to be deleted are marked for deletion by nulling
// out the |func| pointer. We release them at the end of this function
// if we haven't reentered.
gCallbacksInProgress = true;
nsDependentCString prefName(aPrefName);
@@ -4118,17 +4138,17 @@ Preferences::SetPreference(const dom::Pr
if (!pref->HasDefaultValue() && !pref->HasUserValue()) {
gHashTable->RemoveEntry(entry);
}
// Note: we don't have to worry about HandleDirty() because we are setting
// prefs in the content process that have come from the parent process.
if (valueChanged) {
- NotifyCallbacks(prefName);
+ NotifyCallbacks(prefName, PrefWrapper(pref));
}
}
/* static */ void
Preferences::GetPreference(dom::Pref* aDomPref)
{
MOZ_ASSERT(XRE_IsParentProcess());
@@ -4937,17 +4957,17 @@ Preferences::Lock(const char* aPrefName)
Pref* pref = pref_HashTableLookup(aPrefName);
if (!pref) {
return NS_ERROR_UNEXPECTED;
}
if (!pref->IsLocked()) {
pref->SetIsLocked(true);
- NotifyCallbacks(aPrefName);
+ NotifyCallbacks(aPrefName, PrefWrapper(pref));
}
return NS_OK;
}
/* static */ nsresult
Preferences::Unlock(const char* aPrefName)
{
@@ -4956,17 +4976,17 @@ Preferences::Unlock(const char* aPrefNam
Pref* pref = pref_HashTableLookup(aPrefName);
if (!pref) {
return NS_ERROR_UNEXPECTED;
}
if (pref->IsLocked()) {
pref->SetIsLocked(false);
- NotifyCallbacks(aPrefName);
+ NotifyCallbacks(aPrefName, PrefWrapper(pref));
}
return NS_OK;
}
/* static */ bool
Preferences::IsLocked(const char* aPrefName)
{