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.
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
Make it harder for users to accidentally reintroduce usage of the PR_LOG macros
when using 'mozilla/Logging.h'. This can still be worked around by directly
including 'prlog.h' (and not 'mozilla/Logging.h') if absolutely necessary.
This adds the mozilla::LogLevel enum class. Additionaly a log_test function is
added to use rather than a macro, this allows us to enforce only
mozilla::LogLevel is passed into the function.
|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
Make it harder for users to accidentally reintroduce usage of the PR_LOG macros
when using 'mozilla/Logging.h'. This can still be worked around by directly
including 'prlog.h' (and not 'mozilla/Logging.h') if absolutely necessary.
This adds the mozilla::LogLevel enum class. Additionaly a log_test function is
added to use rather than a macro, this allows us to enforce only
mozilla::LogLevel is passed into the function.
Make it harder for users to accidentally reintroduce usage of the PR_LOG macros
when using 'mozilla/Logging.h'. This can still be worked around by directly
including 'prlog.h' (and not 'mozilla/Logging.h') if absolutely necessary.
This adds the mozilla::LogLevel enum class. Additionaly a log_test function is
added to use rather than a macro, this allows us to enforce only
mozilla::LogLevel is passed into the function.
Ryan VanderMeulen
Birunthan Mohanathas
Nicholas Nethercote
Benoit Girard
Ted Mielczarek
Eric Rahm
Wes Kocher
Terrence Cole
Carsten "Tomcat" Book
Botond Ballo