This allows us to have a shared superclass that implements the guts of a shared
superinterface, without having the superclass actually inherit the superinterface
(which leads to annoying and unnecessary diamond-inheritance).
Also, add release asserts that the other methods that addref or release console messages are only run on the main thread.
Finally, add an assert that nsScriptErrorWithStack is only created on the main thread.
The bulk of this commit was generated by running:
run-clang-tidy.py \
-checks='-*,llvm-namespace-comment' \
-header-filter=^/.../mozilla-central/.* \
-fix
Note that enumfunc_pentries and persistent_userstruct are unused, and so could
be removed.
--HG--
extra : rebase_source : 57ae41aa2e7d514dc8f3f3d79d3d1946a407c4ac
This is a particularly nice example of how iterators can be so much nicer than
Enumerate()-style functions:
1 file changed, 4 insertions(+), 33 deletions(-)
--HG--
extra : rebase_source : 757f75b90cb4c624143c236f9743edf158f72d66
nsBaseHashtable has both EnumerateRead() and Enumerate(). A comment claims that
the latter locks the table, but this is false, so I removed the comment. Other
than that the only notable difference between them is that they have slightly
different types for dealing with values (|UserDataType| vs |DataType&|) so I've
implemented both GetUserData() and GetData(), allowing either type to be used.
--HG--
extra : rebase_source : 9d61cc8f4c14082c9f1939ff3ced2b697e043f42
Using g_slice_set_config() fails with newer glib because the slice allocator
now has a static constructor that runs when glib is loaded, consequently
emitting a noisy error message which confuses people into believing it's the
root of their problems.
The only way left to force the slice allocator to use "system" malloc (in
practice, jemalloc) is to set the G_SLICE environment variable to
always-malloc, and that needs to happen before glib is loaded.
Fortunately, the firefox and plugin-container executables don't depend on
glib. Unfortunately, webapprt does, so the problem remains for web apps
running through it. xpcshell and other executables that depend on libxul
directly (as opposed to loading it dynamically) are not covered either.
Adding isMainProcessScriptable() into the middle of nsIInterfaceInfo
caused problems with some binary addons that relied on the ordering of
the methods in nsIInterfaceInfo. In an attempt to placate those addons,
move isMainProcessScriptable() to the end of the vtable. This change is
a no-op for normal libxul usage.
The original motivation for the Iterator/RemovingIterator split was that
PLDHashTable Checker class would treat them differently. But that didn't end up
happening (see bug 1131308). So this patch merges them. This is a small code
size win now but it will become bigger when I add iterators to nsTHashTable and
nsBaseHashtable.
The only complication is that PLDHashTable::Iter() is now non-const, which is
a problem if you use it in a const method. So I added PLDHashTable::ConstIter()
which is used in just two places. It's a bit of a hack -- effectively a
const_cast -- but I don't think it's too bad.
The switch to unsigned integer constants (e.g. "0u") are necessary to avoid
compiler warnings about signed/unsigned comparisons.
--HG--
rename : xpcom/tests/TestPLDHash.cpp => xpcom/tests/gtest/TestPLDHash.cpp
extra : rebase_source : e159d6444581fd0063c5274419ac2126a94607bf
- Its move constructor was moving |aOther.mTable| instead of |aOther|. This
meant that |aOther| wasn't being zeroed out appropriately.
- test_pldhash_RemovingIterator() was testing Iterator's move constructor
instead of RemovingIterator's move constructor, due to a copy/paste
mistake.
--HG--
extra : rebase_source : 1f4880893875218ddb155c76d329e84d884c0432
Using g_slice_set_config() fails with newer glib because the slice allocator
now has a static constructor that runs when glib is loaded, consequently
emitting a noisy error message which confuses people into believing it's the
root of their problems.
The only way left to force the slice allocator to use "system" malloc (in
practice, jemalloc) is to set the G_SLICE environment variable to
always-malloc, and that needs to happen before glib is loaded.
Fortunately, the firefox and plugin-container executables don't depend on
glib. Unfortunately, webapprt does, so the problem remains for web apps
running through it. xpcshell and other executables that depend on libxul
directly (as opposed to loading it dynamically) are not covered either.
Constructing kComponentsInterfaceShimMap required a static constructor
on some compilers, due to a non-constexpr constructor and the necessity
of copying non-constexpr things like nsIID. This static constructor is
large (several kilobytes of object code on x86-64) and completely
unnecessary.
To fix this, let's add a constexpr (well, MOZ_CONSTEXPR) constructor to
ComponentsInterfaceShimEntry. This change alone doesn't completely
solve our problem, because the nsIID member still needs to be copied.
But doing that copying is silly: we only use the IID for constructing a
ShimInterfaceInfo in ShimInterfaceInfo::MaybeConstruct, and the
ShimInterfaceInfo constructor takes a const reference. So let's store a
const reference in ComponentsInterfaceShimEntry, too, and make that
structure significantly smaller in the process.
PostTimerEvent is only called by the timer thread, which is already able
to access private members of nsTimerImpl; there's no reason for
PostTimerEvent to be public.
GetTracedTask() is only called from nsTimerImpl itself, so it doesn't
need to be public. GetTLSTraceInfo() is called from the timer thread,
which has access to our private members already.
This patch factors out the existing capacity calculation code in HashShift()
into a new function called BestCapacity(), and then reuses it for
post-enumeration shrinking.
BestCapacity() computes capacity with |CeilingLog2(ceil(length * 4 / 3))|,
which ensures a minimum capacity while respecting the "max 75% full" and
"capacity is a power of two" constraints. In contrast, the old post-enumeration
shrink calculation was |CeilingLog2(length + length/2)|, which gives higher
results in some cases. (Both calculations also ensured the capacity wasn't too
small.) E.g. if length is 48, the former calculation will give 64, while the
latter will give 128.
Therefore, post-enumeration shrinking will no longer give a
larger-than-necessary capacity some cases. This feels like the right thing to
do in isolation, and making it consistent with HashShift() -- used during table
construction -- is also good.
--HG--
extra : rebase_source : 55e982b601c345d10da7abd03a13aec3f5b61598
This change splits PLDHashTable::Iterator::NextEntry() into two separate
functions, which allow you to get the current element and advance the iterator
separately, which means you can use a for-loop to iterate instead of a
while-loop.
As part of this change, the internals of PLDHashTable::Iterator were
significantly changed and simplified (and modelled after js::HashTable's
equivalent code). It's no longer duplicating code from PL_DHashTableEnumerator.
The chaos mode code was a casualty of this, but given how unreliable that code
has proven to be (see bug 1173212, bug 1174046) this is for the best. (We can
reimplement chaos mode once PLDHashTable::Iterator is back on more solid
footing again, if we think it's important.)
All these changes will make it much easier to add an alternative Iterator that
removes elements, which was turning out to be difficult with the prior code.
In order to make the for-loop header usually fit on a single line, I
deliberately renamed a bunch of things to have shorter names.
In summary, you used to write this:
PLDHashTable::Iterator iter(&table);
while (iter.HasMoreEntries()) {
auto entry = static_cast<FooEntry*>(iter.NextEntry());
// ... do stuff with |entry| ...
}
// iter's scope extends beyond here
and now you write this:
for (auto iter = table.Iter(); !iter.Done(); iter.Next()) {
auto entry = static_cast<FooEntry*>(iter.Get());
// ... do stuff with |entry| ...
}
// iter's scope doesn't reach here
--HG--
extra : rebase_source : fa5cac2fc50b1ab7624030bced4763131280f4d8
If you use PLDHashTable::Iterator in chaos mode with a table with zero
capacity, a |% 0| operation takes place in randomUint32LessThan. This change
avoids that.
--HG--
extra : rebase_source : 85f2affb57c2402f40f3d117434b8300e7f204b7
This change reimplements nsTHashtable::Clear() using PLDHashable::Clear(). This
changes its semantics slightly -- the old version would clear the table but
leave its capacity unchanged. The new version will adjust the capacity
to the default, though the entry storage will only be re-allocated when the
first new element is added.
The old code attempted to deal with any OOMs during this enumeration --
OOMs are possible because it's growing an nsCOMArray -- but failed to do so
correctly.
- It didn't check the return value of AppendObject().
- It did check that EntryCount() matched the return value of
PL_DHashTableEnumerate(), but that's always (and vacuously) true.
The new code just returns NS_ERROR_OUT_OF_MEMORY if AppendObject() fails; this
is trivial now that it uses an iterator and doesn't have to call out to another
function.
