The patch removes 455 occurrences of FAIL_ON_WARNINGS from moz.build files, and
adds 78 instances of ALLOW_COMPILER_WARNINGS. About half of those 78 are in
code we control and which should be removable with a little effort.
--HG--
extra : rebase_source : 82e3387abfbd5f1471e953961d301d3d97ed2973
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
The bulk of this commit was generated by running:
run-clang-tidy.py \
-checks='-*,llvm-namespace-comment' \
-header-filter=^/.../mozilla-central/.* \
-fix
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
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
|mOps| is always non-null now, and there's no longer any distinction between
and uninitialized and initialized table. Yay.
--HG--
extra : rebase_source : 3d1fb72aee4dd21ff20db0ff3166d4e932ade897
This is a temporary sub-class of PLDHashTable that will allow PLDHashTable to
be incrementally transitioned from manual initialization/finalization (via
explicit Init()/Fini() calls) to automatic initialization/finalization (via an
initializing constructor and a destructor). Once all PLDHashTable instances are
converted to PLDHashTable2, it can be folded back into PLDHashTable and the "2"
suffix can be dropped.
--HG--
extra : rebase_source : 674e7bd9320dc1db8879f842df05a7d995069e97
Due to Android startup regressions (bug 1163066) and plugin crashes (bug
1165155).
--HG--
extra : rebase_source : 380f79e67dff4c4eaa2614f286a4d0669666b652
This patch converts easy cases, i.e. where the PL_DHashTableInit() call occurs
in a constructor and the PL_DHashTableFinish() call occurs in a destructor.
This fixes the following problems with PLDHashTable::operator=:
- It doesn't handle self-assigments.
- It leaks the memory used by the assigned-to table.
- It doesn't leave the assigned-from table in a safely destructable state.
--HG--
extra : rebase_source : 433ac3418c00e3bb6d376982e4c679d27e42a377
They are kept around for the sake of the standalone glue, which is used
for e.g. webapprt, which doesn't have direct access to jemalloc, and thus
still needs a wrapper to go through the xpcom function list and get to
jemalloc from there.
It's no longer needed now that entry storage isn't allocated there. (The other
possible causes of failures in that function are less interesting and simply
crashing is a reasonable thing to do for them.)
This also makes PL_DNewHashTable() infallible, so I removed some
now-unnecessary checks of its result.
--HG--
extra : rebase_source : 4c6ab0c449bc18e8bace8bf036b5bd78d3a2f1c4
This makes zero-element hash tables, which are common, smaller, and also avoids
unnecessary malloc/free pairs.
I did some measurements during some basic browsing of a few sites. I found that
35% of all live tables were empty with a few tabs open. And cumulatively, for
the whole session, 45% of tables never had an element added to them.
--HG--
extra : rebase_source : 306bb50f250c09aa03a5e4822f41d6f605d76a1d
I kept all the existing PL_DHashTableAdd() calls fallible, in order to be
conservative, except for the ones in nsAtomTable.cpp which already were
followed immediately by an abort on failure.
--HG--
extra : rebase_source : 526d96ab65e4d7d71197b90d086d19fbdd79b7b5
I kept all the existing PL_DHashTableAdd() calls fallible, in order to be
conservative, except for the ones in nsAtomTable.cpp which already were
followed immediately by an abort on failure.
--HG--
extra : rebase_source : eeba14d732077ef2e412f4caca852de6b6b85f55
Because they are now just equivalent to |new PLDHashTable()| +
PL_DHashTableInit() and PL_DHashTableFinish(t) + |delete t|, respectively.
They're only used in a handful of places and obscure things more than they
clarify -- I only recently worked out exactly how they different from Init()
and Finish().
--HG--
extra : rebase_source : c958491447523becff3e01de45a5d2d227d1ecd3
Because it's no longer needed now that entry storage isn't allocated there.
(The other possible causes of failures are much less interesting and simply
crashing is a reasonable thing to do for them.)
This also makes PL_DNewHashTable() infallible.
--HG--
extra : rebase_source : 848cc9bbdfe434525857183b8370d309f3acbf49
This makes zero-element hash tables, which are common, smaller, and also avoids
unnecessary malloc/free pairs.
I did some measurements during some basic browsing of a few sites. I found that
35% of all live tables were empty with a few tabs open. And cumulatively, for
the whole session, 45% of tables never had an element added to them.
There is more to be done w.r.t. simplifying initialization, which will occur in
the next patch.
--HG--
extra : rebase_source : b9bfdcd680f39f3c947a49ae8462c04bc5e38805
Currently the setting of PLDHashTable::ops is very haphazard.
- PLDHashTable has no constructor, so it's not auto-nulled, so lots of places
null it themselves.
- In the fallible PLDHashTable::Init() function, if the entry storage
allocation fails we'll be left with a table that has |ops| set -- indicating
it's been initialized -- but has null entry storage. I'm not certain this can
cause problems but it feels unsafe, and some (but not all) callers of Init()
null it on failure.
- PLDHashTable does not null |ops| in Finish(), so some (but not all) callers
do this themselves.
This patch makes things simpler.
- It adds a constructor that zeroes |ops|.
- It modifies Init() so that it only sets |ops| once success is ensured.
- It zeroes |ops| in Finish().
- Finally, it removes all the now-unnecessary |ops| nulling done by the users
of PLDHashTable.
--HG--
extra : rebase_source : bb34979c218d152562a2f9c7e5215256c111cc5b
ReplaceSubstring() is an O(n*m) algorithm (n being the length of the
string and m being the number of occurrences of aTarget) because we have
to move the remainder of the string, search it again and potentially
memmove most of it again as we find more matches. This patch rewrites
that function to make it O(n+m).
Note that we currently don't build TestStrings.cpp, so the test case in
this patch is not run automatically, but the test case has been verified
to pass separately by moving the test function into Gecko and calling it
during startup and stepping through it in the debugger.
--HG--
extra : rebase_source : b020e17c1973330b0dbbd6bf956c073cfdcb775e
ReplaceSubstring() is an O(n*m) algorithm (n being the length of the
string and m being the number of occurrences of aTarget) because we have
to move the remainder of the string, search it again and potentially
memmove most of it again as we find more matches. This patch rewrites
that function to make it O(n+m).
Note that we currently don't build TestStrings.cpp, so the test case in
this patch is not run automatically, but the test case has been verified
to pass separately by moving the test function into Gecko and calling it
during startup and stepping through it in the debugger.
ReplaceSubstring() is an O(n*m) algorithm (n being the length of the
string and m being the number of occurrences of aTarget) because we have
to move the remainder of the string, search it again and potentially
memmove most of it again as we find more matches. This patch rewrites
that function to make it O(n+m).
Note that we currently don't build TestStrings.cpp, so the test case in
this patch is not run automatically, but the test case has been verified
to pass separately by moving the test function into Gecko and calling it
during startup and stepping through it in the debugger.
--HG--
extra : rebase_source : 0330c130520802392b92bd094dde85f57cfe6420
xpcom/tests/unit/test_nsIProcess.js and
xpcom/tests/unit/test_nsIProcess_stress.js were failing with this commit
applied. Backout out.
CLOSED TREE
--HG--
extra : amend_source : 79cda414c263d97e77309da6ec6d58718b91d199
This also effectively changes how DMD is enabled from requiring both
replace-malloc initialization and the DMD environment variable to
requiring only the former. The DMD environment variable can still be
used to specify options, but not to disable entirely.
This however doesn't touch all the parts that do enable DMD by setting
the DMD environment variable to 1, so the code to handle this value
is kept.
There are, sadly, many combinations of linkage in use throughout the tree.
The main differentiator, though, is between program/libraries related to
Gecko or not. Kind of. Some need mozglue, some don't. Some need dependent
linkage, some standalone.
Anyways, these new templates remove the need to manually define the
right dependencies against xpcomglue, nspr, mozalloc and mozglue
in most cases.
Places that build programs and were resetting MOZ_GLUE_PROGRAM_LDFLAGS
or that build libraries and were resetting MOZ_GLUE_LDFLAGS can now
just not use those Gecko-specific templates.
Ben Kelly
Honza Bambas
Wes Kocher
Andrew McCreight
Eric Rahm
Andrew McCreight
Mike Hommey
Nicholas Nethercote
Xidorn Quan
Nigel Babu
Botond Ballo
James Cheng
Byron Campen [:bwc]
Bobby Holley
Birunthan Mohanathas
Dragana Damjanovic
Ryan VanderMeulen
Nikhil Marathe
Brian O'Keefe
JW Wang
Ehsan Akhgari
Benjamin Smedberg
Nick Fitzgerald
Denis Volk
Andrea Marchesini
Geoff Brown
Christoph Kerschbaumer
Phil Ringnalda
Michael Pruett
Chris Peterson
Denis Volk
Gregory Szorc
Aaron Klotz
Benjamin Bouvier
Brian Marshall