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.
It feels safer to use a function with a new name, rather than just changing the
behaviour of the existing function.
For most of these cases the PL_DHashTableLookup() result was checked with
PL_DHASH_ENTRY_IS_{FREE,BUSY} so the conversion was easy. A few of them
preceded that check with a useless null check, but the intent of these was
still easy to determine.
I'll do the trickier ones in subsequent patches.
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.
This patch does the following.
- Moves the logic for computing the ideal capacity for a SegmentedArray out of
SnowWhiteKiller into its own class, SegmentedArrayCapacity.
- Replaces the nsTArray in CollectWhite(), which can be very large and is
complicit in ~1% of OOM crashes, with a SegmentedArray.
This patch generalizes SegmentedArray a little, and then uses it instead of
nsTArray in SnowWhiteKiller. This avoids some large (sometimes 1 MiB or more)
allocations which were usually mostly unused.
Root() does not actually root JS things, so if some other class's Unlink() method ends
up calling the GC, whiteNodes will end up containing dead pointers. (This is safe right
now because the Unlink and Unroot methods do not do anything to JS things.) It is less
error prone to simply never store those pointers.
Also, add some asserts to enforce that we never call any of the white-object methods
for JS things.
If an Unlink() method ends up running JS, it can cause a GC, which will make us reenter the CC,
which will not do anything because we're already in a CC. Therefore, FinishAnyCurrentCollection()
won't finish the CC. This is safe because the CC only touches things it actually holds alive via
the Root() method.
