This can cause leaks that are invisible to our XPCOM leak detection system.
To avoid this, classes should not addref or release in their Traverse methods.
--HG--
extra : rebase_source : acd0b070c63cbb4111c165d6b131b8e3b822773a
After this change, we have PLDHashTable::ShallowSizeOf{In,Ex}cludingThis(),
which don't do anything to measure children. (They can be combined with
iteration to measure children.)
This patch also removes the PL_DHashTableSizeOf{In,Ex}cludingThis() functions.
They're not necessary because the methods can be used instead.
Finally, the patch deliberately converts some SizeOfExcludingThis() calls to
SizeOfIncludingThis(). These are all done on heap pointers so this change is
valid.
--HG--
extra : rebase_source : b1d51096a8e7dcac29d7efd92e28938836ff5481
This makes it clearer that, unlike how SizeOf*() functions usually work, this
doesn't measure any children hanging off the array.
And do likewise for nsTObserverArray.
--HG--
extra : rebase_source : 6a8c8d8ffb53ad51b5773afea77126cdd767f149
StorensRefPtrPassByPtr is currently used for storing reference-counted
types passed as T* template arguments to NS_NewRunnableMethodWith*.
We'd also like to use it to store nsRefPtr<T> template arguments. While
it could be used in its current form, it'd be better for its constructor
to support forwarding, so that something like:
NS_NewRunnableMethodWithArg<nsRefPtr<T>>(..., nsRefPtr<T>(local));
doesn't cause unnecessary reference counting.
CLOSED TREE
--HG--
extra : amend_source : 0b7ac18429b248cf05cfe33f6b2f6efdf1602c38
extra : histedit_source : bdb11dafa100809ec17491e5711fb0b507e023c6%2C5e4134650a804859dc8b3078688fa4655052263f
nsCheapSet is used little enough that I didn't bother creating an iterator for
it.
I removed the dependency on PLDHashOperator by introducing nsCheapSetOperator,
which is equivalent.
--HG--
extra : rebase_source : 8a15ae2ee0949a241f6417bfab614affbec2987c
nsDiscriminatedUnion owns memory without using smart pointers, so implementing anything
that would copy or move around one of these will require some care. Just forbid these
for now.
The existing nsDiscriminateUnions either always call Cleanup() when they
are about to go away, or they only handle scalar values so it is safe to
call Cleanup() on them without worrying about another discriminated union
having taken over any memory owned by this union.
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.
Iterator::NextEntry() miscomputes |entryLimit|. This doesn't matter in
non-chaos mode because we'll always find a live entry before hitting that
limit. But it does matter in chaos mode because it means we don't wrap around
when we should.
It's clear how this mistake was made -- the code from Enumerate() was copied.
In Enumerate() |mEntryStore| and |entryAddr| are the same when |entryLimit| is
computed, so you can use them interchangeably. But in NextEntry() |mEntryAddr|
will have moved past |mEntryStore|, so you have to use |mEntryStore|. I changed
both functions in the same way to keep the correspondence between them obvious.
--HG--
extra : rebase_source : f27558b3179be394526d1c3f82ffbae0fb58b2b9
Jesse Ruderman
Kyle Huey
Andrew McCreight
Nicholas Nethercote
James Cheng
Byron Campen [:bwc]
Aidin Gharibnavaz
Bobby Holley
Nathan Froyd
Ryan VanderMeulen
Jim Blandy
Carsten "Tomcat" Book
Jacek Caban
Ehsan Akhgari
Honza Bambas
Michael Layzell
Xidorn Quan
Julian Seward
Nate Hughes
Birunthan Mohanathas
Victor Porof
Wes Kocher
Benoit Girard
Kan-Ru Chen
Ben Kelly
Peter Elmers
Jeff Muizelaar
Olli Pettay
Alexandre Poirot
Jim Mathies
Randell Jesup
Mike Hommey
Dragana Damjanovic
Robert O'Callahan
Bill McCloskey
Gabriele Svelto
Andrea Marchesini
Fabrice Desré
Juan Gomez
Phil Ringnalda
Hamzata Diallo
Boris Kudryavtsev
William Chen
Jan de Mooij
ffxbld
David Major
Chris Peterson
Tom Tromey
Boris Zbarsky
Shelly Lin
Florian Quèze
Terrence Cole
Ted Mielczarek
Eric Rahm
Nick Fitzgerald