mozilla
/
gecko-dev
зеркало из https://github.com/mozilla/gecko-dev.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Bug 1688833 - Change remaining references to LookupForAdd to WithEntryHandle and remove LookupForAdd. r=xpcom-reviewers,nika 

Differential Revision: https://phabricator.services.mozilla.com/D104400
This commit is contained in:
Simon Giesecke 2021-02-09 18:19:47 +00:00
Родитель 846d039c3c
Коммит fbfee61619
4 изменённых файлов: 70 добавлений и 191 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

4
dom/base/nsNodeInfoManager.cpp
Просмотреть файл

@ -143,8 +143,8 @@ already_AddRefed<mozilla::dom::NodeInfo> nsNodeInfoManager::GetNodeInfo(
return nodeInfo.forget();
}
// We don't use LookupForAdd here as that would end up storing the temporary
// key instead of using `mInner`.
// We don't use WithEntryHandle here as that would end up storing the
// temporary key instead of using `mInner`.
RefPtr<NodeInfo> nodeInfo = mNodeInfoHash.Get(&tmpKey);
if (!nodeInfo) {
++mNonDocumentNodeInfos;

4
dom/serviceworkers/ServiceWorkerManager.cpp
Просмотреть файл

@ -1585,8 +1585,8 @@ ServiceWorkerManager::GetOrCreateJobQueue(const nsACString& aKey,
const nsACString& aScope) {
MOZ_ASSERT(!aKey.IsEmpty());
ServiceWorkerManager::RegistrationDataPerPrincipal* data;
// XXX we could use LookupForAdd here to avoid a hashtable lookup, except that
// leads to a false positive assertion, see bug 1370674 comment 7.
// XXX we could use WithEntryHandle here to avoid a hashtable lookup, except
// that leads to a false positive assertion, see bug 1370674 comment 7.
if (!mRegistrationInfos.Get(aKey, &data)) {
data = new RegistrationDataPerPrincipal();
mRegistrationInfos.Put(aKey, data);

99
xpcom/ds/nsBaseHashtable.h
Просмотреть файл

@ -193,7 +193,7 @@ class nsBaseHashtable
* constructed.
*
* This function can only be used if DataType is default-constructible. Use
* LookupForAdd with non-default-constructible DataType for now.
* WithEntryHandle with non-default-constructible DataType for now.
*
* TODO: Add a function GetOrInsertWith that will use a function for
* DataType construction.
@ -376,107 +376,12 @@ class nsBaseHashtable
* This is useful for cases where you want to read/write the value of an entry
* and (optionally) remove the entry without having to do multiple hashtable
* lookups. If you want to insert a new entry if one does not exist, then use
* LookupForAdd instead, see below.
* WithEntryHandle instead, see below.
*/
[[nodiscard]] LookupResult Lookup(KeyType aKey) {
return LookupResult(this->GetEntry(aKey), *this);
}
struct EntryPtr {
private:
EntryType* mEntry;
bool mExistingEntry;
nsBaseHashtable& mTable;
// For debugging purposes
#ifdef DEBUG
uint32_t mTableGeneration;
bool mDidInitNewEntry;
#endif
public:
EntryPtr(nsBaseHashtable& aTable, EntryType* aEntry, bool aExistingEntry)
: mEntry(aEntry),
mExistingEntry(aExistingEntry),
mTable(aTable)
#ifdef DEBUG
,
mTableGeneration(aTable.GetGeneration()),
mDidInitNewEntry(false)
#endif
{
}
~EntryPtr() {
MOZ_ASSERT(mExistingEntry || mDidInitNewEntry || !mEntry,
"Forgot to call OrInsert() or OrRemove() on a new entry");
}
// Is there something stored in the table already?
explicit operator bool() const {
MOZ_ASSERT(mTableGeneration == mTable.GetGeneration());
return mExistingEntry;
}
template <class F>
DataType& OrInsert(F func) {
MOZ_ASSERT(mTableGeneration == mTable.GetGeneration());
MOZ_ASSERT(mEntry);
if (!mExistingEntry) {
mEntry->mData = Converter::Wrap(func());
#ifdef DEBUG
mDidInitNewEntry = true;
#endif
}
return mEntry->mData;
}
void OrRemove() {
MOZ_ASSERT(mTableGeneration == mTable.GetGeneration());
MOZ_ASSERT(mEntry);
mTable.RemoveEntry(mEntry);
mEntry = nullptr;
}
[[nodiscard]] DataType& Data() {
MOZ_ASSERT(mTableGeneration == mTable.GetGeneration());
MOZ_ASSERT(mEntry);
return mEntry->mData;
}
};
/**
* Looks up aKey in the hashtable and returns an object that allows you to
* insert a new entry into the hashtable for that key if an existing entry
* isn't found for it.
*
* This function can only be used if DataType is default-constructible at the
* moment.
*
* A typical usage of this API looks like this:
*
* auto insertedValue = table.LookupForAdd(key).OrInsert([]() {
* return newValue;
* });
*
* auto p = table.LookupForAdd(key);
* if (p) {
* // The entry already existed in the table.
* DoSomething(p.Data());
* } else {
* // An existing entry wasn't found, store a new entry in the hashtable.
* p.OrInsert([]() { return newValue; });
* }
*
* We ensure that the hashtable isn't modified before EntryPtr method calls.
* This is useful for cases where you want to insert a new entry into the
* hashtable if one doesn't exist before but would like to avoid two hashtable
* lookups.
*/
[[nodiscard]] EntryPtr LookupForAdd(KeyType aKey) {
auto count = Count();
EntryType* ent = this->PutEntry(aKey);
return EntryPtr(*this, ent, count == Count());
}
/**
* Used by WithEntryHandle as the argument type to its functor. It is
* associated with the Key passed to WithEntryHandle and manages only the

154
xpcom/tests/gtest/TestHashtables.cpp
Просмотреть файл

@ -794,52 +794,6 @@ TYPED_TEST_P(BaseHashtableTest, Lookup_Remove) {
res.Remove();
}
TYPED_TEST_P(BaseHashtableTest, LookupForAdd) {
// The old LookupForAdd function doesn't support non-default-constructible
// DataType at the moment. It will be addressed in bug 1688833.
if constexpr (std::is_default_constructible_v<typename TypeParam::DataType>) {
auto table = MakeBaseHashtable<TypeParam>(
TypeParam::kExpectedAddRefCnt_LookupForAdd);
auto res = table.LookupForAdd(1);
EXPECT_TRUE(res);
}
}
TYPED_TEST_P(BaseHashtableTest, LookupForAdd_OrInsert) {
// The old LookupForAdd function doesn't support non-default-constructible
// DataType at the moment. It will be addressed in bug 1688833.
if constexpr (std::is_default_constructible_v<typename TypeParam::DataType>) {
auto table = MakeEmptyBaseHashtable<TypeParam>();
auto res = table.LookupForAdd(1);
EXPECT_FALSE(res);
res.OrInsert([] {
return typename TypeParam::UserDataType(MakeRefPtr<TestUniCharRefCounted>(
42, TypeParam::kExpectedAddRefCnt_LookupForAdd_OrInsert));
});
}
}
TYPED_TEST_P(BaseHashtableTest, LookupForAdd_OrRemove) {
// The old LookupForAdd function doesn't support non-default-constructible
// DataType at the moment. It will be addressed in bug 1688833.
if constexpr (std::is_default_constructible_v<typename TypeParam::DataType>) {
auto table = MakeEmptyBaseHashtable<TypeParam>();
auto res = table.LookupForAdd(1);
EXPECT_FALSE(res);
res.OrInsert([] {
return typename TypeParam::UserDataType(MakeRefPtr<TestUniCharRefCounted>(
42, TypeParam::kExpectedAddRefCnt_LookupForAdd_OrRemove));
});
res.OrRemove();
}
}
TYPED_TEST_P(BaseHashtableTest, WithEntryHandle_NoOp) {
auto table = MakeEmptyBaseHashtable<TypeParam>();
@ -998,7 +952,6 @@ REGISTER_TYPED_TEST_CASE_P(
SizeOfIncludingThis, Count, IsEmpty, Get_OutputParam, Get, MaybeGet,
GetOrInsert, Put, Put_Fallible, Put_Rvalue, Put_Rvalue_Fallible,
Remove_OutputParam, Remove, GetAndRemove, RemoveIf, Lookup, Lookup_Remove,
LookupForAdd, LookupForAdd_OrInsert, LookupForAdd_OrRemove,
WithEntryHandle_NoOp, WithEntryHandle_NotFound_OrInsert,
WithEntryHandle_NotFound_OrInsertFrom,
WithEntryHandle_NotFound_OrInsertFrom_Exists,
@ -1289,21 +1242,25 @@ TEST(Hashtables, InterfaceHashtable)
ASSERT_EQ(count, uint32_t(0));
}
TEST(Hashtables, DataHashtable_LookupForAdd)
TEST(Hashtables, DataHashtable_WithEntryHandle)
{
// check LookupForAdd/OrInsert
// check WithEntryHandle/OrInsertWith
nsDataHashtable<nsUint32HashKey, const char*> UniToEntity(ENTITY_COUNT);
for (auto& entity : gEntities) {
auto entry = UniToEntity.LookupForAdd(entity.mUnicode);
const char* val = entry.OrInsert([&entity]() { return entity.mStr; });
ASSERT_FALSE(entry);
ASSERT_TRUE(val == entity.mStr);
ASSERT_TRUE(entry.Data() == entity.mStr);
UniToEntity.WithEntryHandle(entity.mUnicode, [&entity](auto&& entry) {
EXPECT_FALSE(entry);
const char* const val =
entry.OrInsertWith([&entity]() { return entity.mStr; });
EXPECT_TRUE(entry);
EXPECT_TRUE(val == entity.mStr);
EXPECT_TRUE(entry.Data() == entity.mStr);
});
}
for (auto& entity : gEntities) {
ASSERT_TRUE(UniToEntity.LookupForAdd(entity.mUnicode));
UniToEntity.WithEntryHandle(entity.mUnicode,
[](auto&& entry) { EXPECT_TRUE(entry); });
}
// 0 should not be found
@ -1321,7 +1278,8 @@ TEST(Hashtables, DataHashtable_LookupForAdd)
ASSERT_TRUE(count == UniToEntity.Count());
for (auto& entity : gEntities) {
ASSERT_TRUE(UniToEntity.LookupForAdd(entity.mUnicode));
UniToEntity.WithEntryHandle(entity.mUnicode,
[](auto&& entry) { EXPECT_TRUE(entry); });
}
// Lookup().Remove() should remove all entries.
@ -1334,45 +1292,53 @@ TEST(Hashtables, DataHashtable_LookupForAdd)
// Remove newly added entries via OrRemove.
for (auto& entity : gEntities) {
auto entry = UniToEntity.LookupForAdd(entity.mUnicode);
ASSERT_FALSE(entry);
entry.OrRemove();
UniToEntity.WithEntryHandle(entity.mUnicode, [](auto&& entry) {
EXPECT_FALSE(entry);
entry.OrRemove();
});
}
ASSERT_TRUE(0 == UniToEntity.Count());
// Remove existing entries via OrRemove.
for (auto& entity : gEntities) {
auto entry = UniToEntity.LookupForAdd(entity.mUnicode);
const char* val = entry.OrInsert([&entity]() { return entity.mStr; });
ASSERT_FALSE(entry);
ASSERT_TRUE(val == entity.mStr);
ASSERT_TRUE(entry.Data() == entity.mStr);
UniToEntity.WithEntryHandle(entity.mUnicode, [&entity](auto&& entry) {
EXPECT_FALSE(entry);
const char* const val = entry.OrInsert(entity.mStr);
EXPECT_TRUE(entry);
EXPECT_TRUE(val == entity.mStr);
EXPECT_TRUE(entry.Data() == entity.mStr);
});
auto entry2 = UniToEntity.LookupForAdd(entity.mUnicode);
ASSERT_TRUE(entry2);
entry2.OrRemove();
UniToEntity.WithEntryHandle(entity.mUnicode, [](auto&& entry) {
EXPECT_TRUE(entry);
entry.OrRemove();
});
}
ASSERT_TRUE(0 == UniToEntity.Count());
}
TEST(Hashtables, ClassHashtable_LookupForAdd)
TEST(Hashtables, ClassHashtable_WithEntryHandle)
{
// check a class-hashtable LookupForAdd with null values
// check a class-hashtable WithEntryHandle with null values
nsClassHashtable<nsCStringHashKey, TestUniChar> EntToUniClass(ENTITY_COUNT);
for (auto& entity : gEntities) {
auto entry = EntToUniClass.LookupForAdd(nsDependentCString(entity.mStr));
const TestUniChar* val = entry.OrInsert([]() { return nullptr; }).get();
ASSERT_FALSE(entry);
ASSERT_TRUE(val == nullptr);
ASSERT_TRUE(entry.Data() == nullptr);
EntToUniClass.WithEntryHandle(
nsDependentCString(entity.mStr), [](auto&& entry) {
EXPECT_FALSE(entry);
const TestUniChar* val = entry.OrInsert(nullptr).get();
EXPECT_TRUE(entry);
EXPECT_TRUE(val == nullptr);
EXPECT_TRUE(entry.Data() == nullptr);
});
}
for (auto& entity : gEntities) {
ASSERT_TRUE(EntToUniClass.LookupForAdd(nsDependentCString(entity.mStr)));
ASSERT_TRUE(
EntToUniClass.LookupForAdd(nsDependentCString(entity.mStr)).Data() ==
nullptr);
EntToUniClass.WithEntryHandle(nsDependentCString(entity.mStr),
[](auto&& entry) { EXPECT_TRUE(entry); });
EntToUniClass.WithEntryHandle(
nsDependentCString(entity.mStr),
[](auto&& entry) { EXPECT_TRUE(entry.Data() == nullptr); });
}
// "" should not be found
@ -1390,7 +1356,8 @@ TEST(Hashtables, ClassHashtable_LookupForAdd)
ASSERT_TRUE(count == EntToUniClass.Count());
for (auto& entity : gEntities) {
ASSERT_TRUE(EntToUniClass.LookupForAdd(nsDependentCString(entity.mStr)));
EntToUniClass.WithEntryHandle(nsDependentCString(entity.mStr),
[](auto&& entry) { EXPECT_TRUE(entry); });
}
// Lookup().Remove() should remove all entries.
@ -1403,23 +1370,30 @@ TEST(Hashtables, ClassHashtable_LookupForAdd)
// Remove newly added entries via OrRemove.
for (auto& entity : gEntities) {
auto entry = EntToUniClass.LookupForAdd(nsDependentCString(entity.mStr));
ASSERT_FALSE(entry);
entry.OrRemove();
EntToUniClass.WithEntryHandle(nsDependentCString(entity.mStr),
[](auto&& entry) {
EXPECT_FALSE(entry);
entry.OrRemove();
});
}
ASSERT_TRUE(0 == EntToUniClass.Count());
// Remove existing entries via OrRemove.
for (auto& entity : gEntities) {
auto entry = EntToUniClass.LookupForAdd(nsDependentCString(entity.mStr));
const TestUniChar* val = entry.OrInsert([]() { return nullptr; }).get();
ASSERT_FALSE(entry);
ASSERT_TRUE(val == nullptr);
ASSERT_TRUE(entry.Data() == nullptr);
EntToUniClass.WithEntryHandle(
nsDependentCString(entity.mStr), [](auto&& entry) {
EXPECT_FALSE(entry);
const TestUniChar* val = entry.OrInsert(nullptr).get();
EXPECT_TRUE(entry);
EXPECT_TRUE(val == nullptr);
EXPECT_TRUE(entry.Data() == nullptr);
});
auto entry2 = EntToUniClass.LookupForAdd(nsDependentCString(entity.mStr));
ASSERT_TRUE(entry2);
entry2.OrRemove();
EntToUniClass.WithEntryHandle(nsDependentCString(entity.mStr),
[](auto&& entry) {
EXPECT_TRUE(entry);
entry.OrRemove();
});
}
ASSERT_TRUE(0 == EntToUniClass.Count());
}