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.
The destructor is "opt-in" -- there's a flag that makes it a no-op unless the
table was initialized with the initializing constructor. This will allow us to
incrementally convert existing tables from manual to automatic
initialization/finalization. This is important because some of the existing
uses are tricky (impossible?) to convert to the automatic style.
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.
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.
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.
The use of |new| in PL_NewDHashTable() is necessary to avoid the new
assertions in Init() from failing.
--HG--
extra : rebase_source : 63cf962ce146142b72ffa0d6fcd3d8af1ec88bca
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
PL_DHashTableLookup() had the same return protocol as SearchTable(), which
meant that compilers could generate a tail call from the former to the latter.
Bug 1124973 replaced PL_DHashTableLookup() with PL_DHashTableSearch(), which
has a slightly different return protocol, and so the tail call was lost. This
appears to be the cause of the Fennec performance regression.
This patch splits SearchTable() in two (using templates to avoid explicit code
duplication). SearchTable<false>() now has the same return protocol as
PL_DHashTableSearch(), and so the tail call can now be used again.
As well as renaming and privatizing |keyHash|, this patch also:
- renames GetKeyHash() to ComputeKeyHash(), which better indicates it's not
some kind of getter function; and
- makes PLDHashEntryStub inherit from PLDHashEntryHdr, for consistency with
everywhere else.
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.
--HG--
extra : rebase_source : ab37a7a30be563861ded8631771181aacf054fd4
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
