Bug 529808 - Remove the static atom table. r=froydnj
Various atom-related things have improved recently.
- The main atom table is now threadsafe (
bug 1275755) and so can be accessed on
any thread. It has also been split into pieces (
bug 1440824), which greatly
reduces lock contention.
- A cache has been added to the HTML5 parser (
bug 1352874) that removes the
need for most of the full table lookups.
As a result, there is no point having a separate static atom table. This patch
removes it.
MozReview-Commit-ID: 8ou1BrnPAwd
--- a/layout/build/nsLayoutStatics.cpp
+++ b/layout/build/nsLayoutStatics.cpp
@@ -153,17 +153,17 @@ nsLayoutStatics::Initialize()
nsCSSPseudoElements::AddRefAtoms();
nsCSSKeywords::AddRefTable();
nsCSSProps::AddRefTable();
nsColorNames::AddRefTable();
nsGkAtoms::AddRefAtoms();
nsHTMLTags::RegisterAtoms();
nsRDFAtoms::RegisterAtoms();
- NS_SealStaticAtomTable();
+ NS_SetStaticAtomsDone();
StartupJSEnvironment();
rv = nsRegion::InitStatic();
if (NS_FAILED(rv)) {
NS_ERROR("Could not initialize nsRegion");
return rv;
}
--- a/toolkit/components/aboutmemory/tests/test_memoryReporters.xul
+++ b/toolkit/components/aboutmemory/tests/test_memoryReporters.xul
@@ -98,17 +98,17 @@
} else if (aPath.search(/^explicit\/window-objects\/top\(.*\/js-compartment\(/) >= 0) {
present.windowObjectsJsCompartments = true;
} else if (aPath.search(/^explicit\/storage\/sqlite\/places.sqlite/) >= 0) {
present.places = true;
} else if (aPath.search(/^explicit\/images/) >= 0) {
present.images = true;
} else if (aPath.search(/^explicit\/xpti-working-set$/) >= 0) {
present.xptiWorkingSet = true;
- } else if (aPath.search(/^explicit\/atom-tables\/main$/) >= 0) {
+ } else if (aPath.search(/^explicit\/atom-table$/) >= 0) {
present.atomTablesMain = true;
} else if (/\[System Principal\].*this-is-a-sandbox-name/.test(aPath)) {
// A system compartment with a location (such as a sandbox) should
// show that location.
present.sandboxLocation = true;
} else if (aPath.includes(bigStringPrefix)) {
present.bigString = true;
} else if (aPath.includes("!)(*&")) {
@@ -256,17 +256,17 @@
checkSizeReasonable("js-main-runtime-gc-heap-committed/used/gc-things",
jsGcHeapUsedGcThingsTotal);
ok(present.jsNonWindowCompartments, "js-non-window compartments are present");
ok(present.windowObjectsJsCompartments, "window-objects/.../js compartments are present");
ok(present.places, "places is present");
ok(present.images, "images is present");
ok(present.xptiWorkingSet, "xpti-working-set is present");
- ok(present.atomTablesMain, "atom-tables/main is present");
+ ok(present.atomTablesMain, "atom-table is present");
ok(present.sandboxLocation, "sandbox locations are present");
ok(present.bigString, "large string is present");
ok(present.smallString1, "small string 1 is present");
ok(present.smallString2, "small string 2 is present");
ok(!present.anonymizedWhenUnnecessary,
"anonymized paths are not present when unnecessary. Failed case: " +
present.anonymizedWhenUnnecessary);
--- a/xpcom/base/nsMemoryReporterManager.cpp
+++ b/xpcom/base/nsMemoryReporterManager.cpp
@@ -1390,26 +1390,21 @@ class AtomTablesReporter final : public
~AtomTablesReporter() {}
public:
NS_DECL_ISUPPORTS
NS_IMETHOD CollectReports(nsIHandleReportCallback* aHandleReport,
nsISupports* aData, bool aAnonymize) override
{
- size_t Main, Static;
- NS_SizeOfAtomTablesIncludingThis(MallocSizeOf, &Main, &Static);
+ int64_t size = NS_SizeOfAtomTableIncludingThis(MallocSizeOf);
MOZ_COLLECT_REPORT(
- "explicit/atom-tables/main", KIND_HEAP, UNITS_BYTES, Main,
- "Memory used by the main atoms table.");
-
- MOZ_COLLECT_REPORT(
- "explicit/atom-tables/static", KIND_HEAP, UNITS_BYTES, Static,
- "Memory used by the static atoms table.");
+ "explicit/atom-table", KIND_HEAP, UNITS_BYTES, size,
+ "Memory used by the atom table.");
return NS_OK;
}
};
NS_IMPL_ISUPPORTS(AtomTablesReporter, nsIMemoryReporter)
#ifdef DEBUG
--- a/xpcom/ds/nsAtom.h
+++ b/xpcom/ds/nsAtom.h
@@ -160,18 +160,18 @@ already_AddRefed<nsAtom> NS_AtomizeMainT
// Currently this function is only used in tests, which should probably remain
// the case.
nsrefcnt NS_GetNumberOfAtoms();
// Return a pointer for a static atom for the string or null if there's no
// static atom for this string.
nsStaticAtom* NS_GetStaticAtom(const nsAString& aUTF16String);
-// Seal the static atom table.
-void NS_SealStaticAtomTable();
+// Record that all static atoms have been inserted.
+void NS_SetStaticAtomsDone();
class nsAtomString : public nsString
{
public:
explicit nsAtomString(const nsAtom* aAtom) { aAtom->ToString(*this); }
};
class nsAtomCString : public nsCString
--- a/xpcom/ds/nsAtomTable.cpp
+++ b/xpcom/ds/nsAtomTable.cpp
@@ -233,16 +233,22 @@ class nsAtomSubTable
{
friend class nsAtomTable;
Mutex mLock;
PLDHashTable mTable;
nsAtomSubTable();
void GCLocked(GCKind aKind);
size_t SizeOfExcludingThisLocked(MallocSizeOf aMallocSizeOf);
+ AtomTableEntry* Search(AtomTableKey& aKey)
+ {
+ mLock.AssertCurrentThreadOwns();
+ return static_cast<AtomTableEntry*>(mTable.Search(&aKey));
+ }
+
AtomTableEntry* Add(AtomTableKey& aKey)
{
mLock.AssertCurrentThreadOwns();
return static_cast<AtomTableEntry*>(mTable.Add(&aKey)); // Infallible
}
};
// The outer atom table, which coordinates access to the inner array of
@@ -251,16 +257,17 @@ class nsAtomTable
{
public:
nsAtomSubTable& SelectSubTable(AtomTableKey& aKey);
size_t SizeOfIncludingThis(MallocSizeOf aMallocSizeOf);
void GC(GCKind aKind);
already_AddRefed<nsAtom> Atomize(const nsAString& aUTF16String);
already_AddRefed<nsAtom> Atomize(const nsACString& aUTF8String);
already_AddRefed<nsAtom> AtomizeMainThread(const nsAString& aUTF16String);
+ nsStaticAtom* GetStaticAtom(const nsAString& aUTF16String);
void RegisterStaticAtoms(const nsStaticAtomSetup* aSetup, uint32_t aCount);
// The result of this function may be imprecise if other threads are operating
// on atoms concurrently. It's also slow, since it triggers a GC before
// counting.
size_t RacySlowCount();
// This hash table op is a static member of this class so that it can take
@@ -566,59 +573,18 @@ nsAtom::Release()
}
}
return count;
}
//----------------------------------------------------------------------
-class StaticAtomEntry : public PLDHashEntryHdr
-{
-public:
- typedef const nsAString& KeyType;
- typedef const nsAString* KeyTypePointer;
-
- explicit StaticAtomEntry(KeyTypePointer aKey) {}
- StaticAtomEntry(const StaticAtomEntry& aOther) : mAtom(aOther.mAtom) {}
-
- // We do not delete the atom because that's done when gAtomTable is
- // destroyed -- which happens immediately after gStaticAtomTable is destroyed
- // -- in NS_PurgeAtomTable().
- ~StaticAtomEntry() {}
-
- bool KeyEquals(KeyTypePointer aKey) const
- {
- return mAtom->Equals(*aKey);
- }
-
- static KeyTypePointer KeyToPointer(KeyType aKey) { return &aKey; }
- static PLDHashNumber HashKey(KeyTypePointer aKey)
- {
- return HashString(*aKey);
- }
-
- enum { ALLOW_MEMMOVE = true };
-
- // Static atoms aren't really refcounted. Because these entries live in a
- // global hashtable, this reference is essentially owning.
- nsStaticAtom* MOZ_OWNING_REF mAtom;
-};
-
-/**
- * A hashtable of static atoms that existed at app startup. This hashtable
- * helps nsHtml5AtomTable.
- */
-typedef nsTHashtable<StaticAtomEntry> StaticAtomTable;
-static StaticAtomTable* gStaticAtomTable = nullptr;
-
-/**
- * Whether it is still OK to add atoms to gStaticAtomTable.
- */
-static bool gStaticAtomTableSealed = false;
+// Have the static atoms been inserted into the table?
+static bool gStaticAtomsDone = false;
class DefaultAtoms
{
public:
NS_STATIC_ATOM_DECL(empty)
};
NS_STATIC_ATOM_DEFN(DefaultAtoms, empty)
@@ -644,42 +610,33 @@ NS_InitAtomTable()
NS_RegisterStaticAtoms(sDefaultAtomSetup);
}
void
NS_ShutdownAtomTable()
{
MOZ_ASSERT(NS_IsMainThread());
MOZ_ASSERT(gAtomTable);
- delete gStaticAtomTable;
- gStaticAtomTable = nullptr;
#ifdef NS_FREE_PERMANENT_DATA
// Do a final GC to satisfy leak checking. We skip this step in release
// builds.
gAtomTable->GC(GCKind::Shutdown);
#endif
delete gAtomTable;
gAtomTable = nullptr;
}
-void
-NS_SizeOfAtomTablesIncludingThis(MallocSizeOf aMallocSizeOf,
- size_t* aMain, size_t* aStatic)
+size_t
+NS_SizeOfAtomTableIncludingThis(MallocSizeOf aMallocSizeOf)
{
MOZ_ASSERT(NS_IsMainThread());
MOZ_ASSERT(gAtomTable);
- *aMain = gAtomTable->SizeOfIncludingThis(aMallocSizeOf);
-
- // The atoms pointed to by gStaticAtomTable are also pointed to by gAtomTable,
- // and they're measured by the call above. So no need to measure them here.
- *aStatic = gStaticAtomTable
- ? gStaticAtomTable->ShallowSizeOfIncludingThis(aMallocSizeOf)
- : 0;
+ return gAtomTable->SizeOfIncludingThis(aMallocSizeOf);
}
size_t
nsAtomSubTable::SizeOfExcludingThisLocked(MallocSizeOf aMallocSizeOf)
{
mLock.AssertCurrentThreadOwns();
size_t size = mTable.ShallowSizeOfExcludingThis(aMallocSizeOf);
for (auto iter = mTable.Iter(); !iter.Done(); iter.Next()) {
@@ -689,25 +646,18 @@ nsAtomSubTable::SizeOfExcludingThisLocke
return size;
}
void
nsAtomTable::RegisterStaticAtoms(const nsStaticAtomSetup* aSetup,
uint32_t aCount)
{
- // Note: gStaticAtomTable is main-thread-only until the table is sealed,
- // after which it is immutable. So there is no lock protecting it.
MOZ_ASSERT(NS_IsMainThread());
- MOZ_RELEASE_ASSERT(!gStaticAtomTableSealed,
- "Atom table has already been sealed!");
-
- if (!gStaticAtomTable) {
- gStaticAtomTable = new StaticAtomTable();
- }
+ MOZ_RELEASE_ASSERT(!gStaticAtomsDone, "Static atom insertion is finished!");
for (uint32_t i = 0; i < aCount; ++i) {
const char16_t* string = aSetup[i].mString;
nsStaticAtom** atomp = aSetup[i].mAtom;
MOZ_ASSERT(nsCRT::IsAscii(string));
uint32_t stringLen = NS_strlen(string);
@@ -731,23 +681,16 @@ nsAtomTable::RegisterStaticAtoms(const n
"Static atom registration for %s should be pushed back", name.get());
}
atom = static_cast<nsStaticAtom*>(he->mAtom);
} else {
atom = new nsStaticAtom(string, stringLen, hash);
he->mAtom = atom;
}
*atomp = atom;
-
- if (!gStaticAtomTableSealed) {
- StaticAtomEntry* entry =
- gStaticAtomTable->PutEntry(nsDependentAtomString(atom));
- MOZ_ASSERT(atom->IsStaticAtom());
- entry->mAtom = atom;
- }
}
}
void
RegisterStaticAtoms(const nsStaticAtomSetup* aSetup, uint32_t aCount)
{
MOZ_ASSERT(gAtomTable);
gAtomTable->RegisterStaticAtoms(aSetup, aCount);
@@ -885,20 +828,32 @@ int32_t
NS_GetUnusedAtomCount(void)
{
return gUnusedAtomCount;
}
nsStaticAtom*
NS_GetStaticAtom(const nsAString& aUTF16String)
{
- NS_PRECONDITION(gStaticAtomTable, "Static atom table not created yet.");
- NS_PRECONDITION(gStaticAtomTableSealed, "Static atom table not sealed yet.");
- StaticAtomEntry* entry = gStaticAtomTable->GetEntry(aUTF16String);
- return entry ? entry->mAtom : nullptr;
+ MOZ_ASSERT(gStaticAtomsDone, "Static atom setup not yet done.");
+ MOZ_ASSERT(gAtomTable);
+ return gAtomTable->GetStaticAtom(aUTF16String);
+}
+
+nsStaticAtom*
+nsAtomTable::GetStaticAtom(const nsAString& aUTF16String)
+{
+ uint32_t hash;
+ AtomTableKey key(aUTF16String.Data(), aUTF16String.Length(), &hash);
+ nsAtomSubTable& table = SelectSubTable(key);
+ MutexAutoLock lock(table.mLock);
+ AtomTableEntry* he = table.Search(key);
+ return he && he->mAtom->IsStaticAtom()
+ ? static_cast<nsStaticAtom*>(he->mAtom)
+ : nullptr;
}
void
-NS_SealStaticAtomTable()
+NS_SetStaticAtomsDone()
{
MOZ_ASSERT(NS_IsMainThread());
- gStaticAtomTableSealed = true;
+ gStaticAtomsDone = true;
}
--- a/xpcom/ds/nsAtomTable.h
+++ b/xpcom/ds/nsAtomTable.h
@@ -8,12 +8,11 @@
#define nsAtomTable_h__
#include "mozilla/MemoryReporting.h"
#include <stddef.h>
void NS_InitAtomTable();
void NS_ShutdownAtomTable();
-void NS_SizeOfAtomTablesIncludingThis(mozilla::MallocSizeOf aMallocSizeOf,
- size_t* aMain, size_t* aStatic);
+size_t NS_SizeOfAtomTableIncludingThis(mozilla::MallocSizeOf aMallocSizeOf);
#endif // nsAtomTable_h__