Currently lookupOrAdd() will allocate if the table has no storage. But it
doesn't report an error if the allocation fails, which can cause problems.
This patch changes things so that lookupOrAdd() doesn't allocate when the table
has no storage. Instead, it returns an AddPtr that is not *valid* (its mTable
is empty) but it is *live*, and can be used in add(), whereupon the allocation
will occur.
The patch also makes Ptr::isValid() and AddPtr::isValid() non-public, because
the valid vs. live distinction is non-obvious and best kept hidden within the
classes.
--HG--
extra : rebase_source : 95d58725d92cc83332e27a61f98fa61185440e26
Because they each only have a single call site, and the "too many items
removed?" test is already encapsulated within rehashIfOverloaded().
--HG--
extra : rebase_source : f6550c483477c2839764e7f657b542c24c82b1e8
infallibleRehashIfOverloaded() actually matches what it does.
Also, the patch removes the `overloaded()` test within the function, because
`rehashIfOverloaded()` has the same test.
--HG--
extra : rebase_source : 2799e623fbdd4b8bb82f8b83166752e02a0f9202
Specifically:
- checkOverloaded -> rehashIfOverloaded
- checkUnderloaded -> shrinkIfUnderloaded
- checkOverRemoved -> rehashIfOverRemoved
Because I've always found that the `check` prefix doesn't clearly explain that
the table might be changed,
And:
- shouldCompressTable -> overRemoved
Because that matches `overloaded` and `underloaded`.
--HG--
extra : rebase_source : 56e9edd012f4a400ac366895d05ea93fb09ec6b3
Because that's a more accurate description of what it does -- it finds free
*and* removed entries.
--HG--
extra : rebase_source : 72b049f44c61a7406d9691a9d9b6405dd696848c
FunctionTypeTraits< function type > makes it easier to inspect a function's
return type, arity, and parameter types.
It works with free functions, struct/class methods, function objects like
non-generic lambdas and std::function.
Differential Revision: https://phabricator.services.mozilla.com/D2998
--HG--
extra : moz-landing-system : lando
Entry storage allocation now occurs on the first lookupForAdd()/put()/putNew().
This removes the need for init() and initialized(), and matches how
PLDHashTable/nsTHashtable work. It also removes the need for init() functions
in a lot of types that are built on top of mozilla::Hash{Map,Set}.
Pros:
- No need for init() calls and subsequent checks.
- No memory allocated for empty tables, which are not that uncommon.
Cons:
- An extra branch in lookup() and lookupForAdd(), but not in put()/putNew(),
because the existing checkOverloaded() can handle it.
Specifics:
- Construction now can take a length parameter.
- init() is removed. Explicit length-setting, when necessary, now occurs in the
constructors.
- initialized() is removed.
- capacity() now returns zero when the entry storage is absent.
- lookupForAdd() is no longer `const`, because it can instantiate the storage,
which requires modifications.
- lookupForAdd() can now return an invalid AddPtr in two cases:
- old: hashing failure (due to OOM in the hasher)
- new: OOM while instantiating entry storage
The existing failure handling paths for the old case work for the new case.
- clear(), finish(), and clearAndShrink() are replaced by clear(), compact(),
and reserve(). The old compactIfUnderloaded() is also removed.
- Capacity computation code is now in its own functions, bestCapacity() and
hashShift(). setTableSizeLog2() is removed.
- uint32_t is used throughout for capacities, instead of size_t, for
consistency with other similar values.
- changeTableSize() now takes a capacity instead of a deltaLog2, and it can now
handle !mTable.
Measurements:
- Total source code size is reduced by over 900 lines. Also, lots of existing
lines got shorter (i.e. two checks were reduced to one).
- Executable size barely changed, down by 2 KiB on Linux64. The extra branches
are compensated for by the lack of init() calls.
- Speed changed negligibly. The instruction count for Bench_Cpp_MozHash
increased from 2.84 billion to 2.89 billion but any execution time change was
well below noise.
Firefox currently requires gcc 6.1 or later (as of bug 1444274).
MozReview-Commit-ID: CaJwpUgntxn
--HG--
extra : rebase_source : 302d8aa57696e2e96962c67e3497f76e882a472f
extra : source : 2a53a830f79424b2c41c8bdc4816ec24fa943ecc
Bug 1480624 added new code that results in a warning on MSVC debug builds. This
warning is treated as an error and makes such builds unhappy. The warning is
due to implicit deletion of a dtor, this changeset makes that deletion
explicit to avoid the fatal warning.
Differential Revision: https://phabricator.services.mozilla.com/D2865
--HG--
extra : moz-landing-system : lando
Hash{Map,Set}::putNew() can be used on a table that has had elements removed,
despite some comments to the contrary.
This patch fixes those comments. It also clarifies when putNewInfallible() can
be used.
This patch also removes the !isRemoved() assertion in findFreeEntry(), which is
confusing -- !isLive() would be more precise, but also obvious from the
surrounding code.
MozReview-Commit-ID: q4qwKGBsHx
--HG--
extra : rebase_source : 94331879f9a1484159e030de93bd0ab222b54385
Because it's quite strange, badly named, not that useful, and barely used.
Also remove WeakMap::lookupWithDefault(), which is similar, but not used at
all.
MozReview-Commit-ID: IhIl4hQ73U1
--HG--
extra : rebase_source : 7da237a56391836ca5d056248f18bd5e2d8b1564
Because (a) it's kinda weird, and (b) only used in a single test, where it can
be easily replaced with a vanilla add().
MozReview-Commit-ID: L4RoxFb7yGG
--HG--
extra : rebase_source : 515a5ede5d417686907345ad9069c6a41669dd17
It's identical to mozilla::CStringHasher.
Also fix a comment at the top of HashTable.h about CStringHasher.
--HG--
extra : rebase_source : 92176c4f6ea8041f309764b4ce0271a494853a7b
I bet nobody has used them in years. The use of ad hoc profiling (printfs plus
post-processing with a tool like https://github.com/nnethercote/counts) is
generally a better approach.
Bug 1179657 did likewise for PLDHashTable.
--HG--
extra : rebase_source : 57ab26c62f2f1eff2eec827564a3c3ff0af12f01
And add comments delimiting different operation groups. Together these changes
make Hash{Set,Map} much easier to navigate.
--HG--
extra : rebase_source : 21a7da92f898b318a24085b5f0885da85aa3afd1
Again inspired by PLDHashTable, this makes things a bit clearer than passing 0
or sCollisionBit.
It also guarantees more strongly that we'll end up with appropriately
specialized code for the Add vs. NonAdd cases. (That guarantee isn't currently
needed because the compiler inlines things sufficiently anyway, but it can't
hurt.)
--HG--
extra : rebase_source : c2341d7df85f44a00bd1b52eac0e08ac63525196
There are two improvements here.
- When we're just doing a lookup (i.e. aCollisionBit==0), we don't need to
do any special handling of removed entries. (Inlining means that the removed
entry code is entirely removed for lookups.)
- When we're doing an insertion (i.e. aCollisionBit==sCollisionBit), we now
stop adding collision markings once we find a removed entry, because they're
unnecessary after that point.
This change brings the code in alignment with PLDHashTable::SearchTable().
--HG--
extra : rebase_source : 8d6e3a88ac24d0e71a2576997face3bea971c71f
This patch does the following:
- Adds a bunch of useful high-level info at the top of the file, making the
types easier to use for newcomers.
- Adds a comment to every Hash{Set,Map} method that lacked one.
- Tweaks lots of existing comments for clarity.
- Removes comments in Hash{Set,Map} referring to details within HashTable. That
dependence is now covered in the top-level comment.
- Tweaks the signatures in hash policy classes to be more consistent.
--HG--
extra : rebase_source : ab9d6ae38b4b73e12151a228107ad3f3f12e36b3
These basically duplicate the existing Range and Enum classes, but use more
familiar terminology, similar to the iterators we have for
PLDHashTable/nsTHashtable:
- Hash{Set,Map}::all() Hash{Set,Map}::iter()
- Enum constructor Hash{Set,Map}::modIter()
- Range::front() Iterator::get()
- Range::popFront() Iterator::next()
- Range::empty() Iterator::done()
- Enum::mutableFront() ModIterator::getMutable()
- Enum::removeFront() ModIterator::remove()
- Enum::rekeyFront() ModIterator::rekey()
The next patch will reduce the amount of code duplication.
--HG--
extra : rebase_source : 5787756b144bbaea98f381d5a128cb42edb82c41
It's only used by InlineTable::Range, and can be avoided by using
mozilla::Maybe.
This also means Range::mTable can be changed from a pointer to a reference,
like Enum::mTable.
--HG--
extra : rebase_source : 35eb5b0bede361c2c765652e1e4c952f128702ef
In PLDHashTable the equivalent functions have a "Shallow" prefix, which makes
it clear that they don't measure things hanging off the table. This patch makes
mozilla::Hash{Set,Map} do likewise.
MozReview-Commit-ID: 3kwCJynhW7d
--HG--
extra : rebase_source : 9c03d11f376a9fd4cfd5cfcdc0c446c00633b210
This patch converts HashTable.h from SpiderMonkey style to Gecko style.
It was created by first running `./mach clang-format -p mfbt/HashTable.h` to
fix the whitespace, and then doing manual patch-ups, the most significant of
which were:
- Adding braces around single-statement if/else/for/while blocks.
- Renaming class members to `mFoo` form and arguments to `aFoo` form.
- Converting `typedef Old New` to `using New = Old` throughout.
MozReview-Commit-ID: LJ1emPyuAjK
The main change is that the patch copies js/public/HashTable.h to
mfbt/HashTable.h, and then changes it as follows.
- Changes `js` namespaces to `mozilla` (and removes some now-unnecessary
`mozilla::` qualifiers).
- Changes the default AllocPolicy from the SpiderMonkey-specific
`TempAllocPolicy` to the generic `MallocAllocPolicy`.
- Adds `#include "AllocPolicy.h"` (like mfbt/Vector.h).
- Changes `JS_DEBUG` use to `DEBUG`.
- Minor comment updates, as necessary.
js/public/HashTable.h is now tiny, holding just a few renamings of things from
the `mozilla` namespace into the `js` namespace to minimize churn elsewhere.
(Those renamings keep `TempAllocPolicy` as the default AllocPolicy for
js::Hash{Set,Map}.)
Also, various template specializations had to be moved from the `js` namespace
to the `mozilla` namespace to avoid compile errors.
MozReview-Commit-ID: GS9Qn9YeYDA
--HG--
rename : js/public/HashTable.h => mfbt/HashTable.h
Currently we have three ways of representing hash values.
- uint32_t: used in HashFunctions.h.
- PLDHashNumber: defined in PLDHashTable.{h,cpp}.
- js::HashNumber: defined in js/public/Utility.h.
Functions that create hash values with functions from HashFunctions.h use a mix
of these three types. It's a bit of a mess.
This patch introduces mozilla::HashNumber, and redefines PLDHashNumber and
js::HashNumber as synonyms. It also changes HashFunctions.h to use
mozilla::HashNumber throughout instead of uint32_t.
This leaves plenty of places that still use uint32_t that should use
mozilla::HashNumber or one of its synonyms, but I didn't want to tackle that
now.
The patch also:
- Does similar things for the constants defining the number of bits in each
hash number type.
- Moves js::HashNumber from Utility.h to HashTable.h, which is a better spot
for it. (This required changing the signature of ScrambleHashCode(); that's
ok, it'll get moved by the next patch anyway.)
MozReview-Commit-ID: EdoWlCm7OUC
--HG--
extra : rebase_source : 5b92c0c3560eb56850cd8832f8ee514d25e3c16f
We can't use memcmp to compare PODs, largely because of undefined
padding. The rest of the Pod* functions are fine though, since we're
replicating or zeroing PODs.
MozReview-Commit-ID: LSspAi8qCWw
Add a trait method that AutoCleanLinkedList delegates to for calling
delete on non-refcounted list elements.
--HG--
extra : histedit_source : 5e8b05f348d734d9045621d858caed946853fc02
This class implements a shared memory key-value store that fits into a single
memory mapped segment. All of the runtime data for its instances are stored in
the shared memory region, which means that memory overhead for each instance
in each process is only a few bytes.
Importantly, the key and value strings returned by this class are also
pointers into the shared memory region, which means that once an instance is
created, its memory cannot be unmapped until process shutdown.
For the uses I intend to put it to, this is a reasonable constraint. If we
need to use it for shorter-lived maps in the future, we can add an option to
return non-literal dependent strings that will be copied if they need to be
kept alive long term.
MozReview-Commit-ID: 5BwAaDsb7HS
--HG--
extra : rebase_source : b472fe628018f88a2c4d6b3de4b7143aeca55e14
extra : absorb_source : 5cdeb568cfd2b4a5a767191402e699e61e653b3b
Scalar property names (e.g. IntProperty) are already escaped correctly.
This patch escapes the collection property names (e.g. StartObjectProperty)
as well (and adds test coverage for it).
MozReview-Commit-ID: 68kkjPb2ZN7
--HG--
extra : rebase_source : b42939ccebb71fc73914ff595803b2d6c7df2715
This adds 'CorruptionCanaryForStatics', which as the name implies is suitable
for use in objects that are statically declared. It has a trivial destructor
which allows us to avoid the need for static constructors.
--HG--
extra : amend_source : 27f8eff9ead21fde9f5f5d17f16c322d2c995a27
Same approach as the other bug, mostly replacing automatically by removing
'using mozilla::Forward;' and then:
s/mozilla::Forward/std::forward/
s/Forward</std::forward</
The only file that required manual fixup was TestTreeTraversal.cpp, which had
a class called TestNodeForward with template parameters :)
MozReview-Commit-ID: A88qFG5AccP
This was done automatically replacing:
s/mozilla::Move/std::move/
s/ Move(/ std::move(/
s/(Move(/(std::move(/
Removing the 'using mozilla::Move;' lines.
And then with a few manual fixups, see the bug for the split series..
MozReview-Commit-ID: Jxze3adipUh
... to allow a user to remove() an entry, update the values and
re-insert() it into a tree.
MozReview-Commit-ID: GrSY90Q3ugt
--HG--
extra : rebase_source : 9d8876064f9e3d5b9e4249936a4c999b74fcc9ad
When we use RefPtr with nsISupports sub-classes, it's usually because the type
cannot be unambiguously cast to nsISupports. We already have a ToSupports
generic function to resolve ambiguity in these cases, so we may as well use
it here.
MozReview-Commit-ID: FaHhPKAPn6j
--HG--
extra : rebase_source : dd8f3707bdebedfe559aed0caf2c3b0c49163735
extra : histedit_source : 9805787a169329b9c739dfa456cfe6a61a22b7d7
We out-of-line the relevant functions because assertions can generate
quite a bit of code, and we'd rather let the compiler determine if these
functions should be inlined now.
For some reason, GNU as is not happy with the assembly generated after
bug 1238661 anymore on Debian armel.
OTOH, as mentioned in bug 1238661 comment 4, we actually don't need this
workaround anymore, so let's just kill it.
--HG--
extra : rebase_source : 6fd06832136d4f840c65f74b63f1c1bec48d525d
We out-of-line the relevant functions because assertions can generate
quite a bit of code, and we'd rather let the compiler determine if these
functions should be inlined now.
NonDereferenceable denotes the intent that a pointer will (most likely) not be
dereferenced, but its numeric value may be used for e.g. logging purposes.
Dereferencing operations are explicitly disabled to avoid unintentional misuses.
Casting is still possible between related types (same as with raw pointers),
but pointers stay safely stored inside NonDereferenceable objects. These casts
do not trigger `clang++ -fsanitize=vptr` errors.
MozReview-Commit-ID: 5885pB7hSFR
--HG--
extra : rebase_source : 3c4011da64d84f1b19991742b76bafbffa90d590
There are three things we want to be true:
a) If the child sends a large value and the parent can't allocate enough space
for it we use an infallible allocation so the parent dies with an OOM.
b) If a fuzzer generates (huge-length, small-data) we don't try to allocate
huge-length bytes; knowing that the read will fail.
c) No fuzzer-specific branches in the core IPC serialization code.
Finally, this makes (huge-length, small-data) consistent with other cases where
the data is potentially truncated: ReadParam returns false.
MozReview-Commit-ID: 6nDKrw5z4pt
--HG--
extra : rebase_source : 58372d29139e9545a6ed2852c7243affeab6fdb7
Otherwise, one can do thinkos like:
MakeScopeExit(...);
and the scope exiting function will execute much earlier than the
intended:
auto guard = MakeScopeExit(...);
This is a `constexpr` alternative to HashString(const char16_t*). We can't make
HashString(const char16_t*) itself `constexpr` because HashUntilZero(const T*)
isn't in a form that older compilers (like GCC 4.9) allow to be made
`constexpr`. (The trick to satisfying those compilers is to use recursion
instead of iteration, to get the function into a single `return` statement.)
This requires making a bunch of other functions `constexpr` as well. It also
requires adding MOZ_{PUSH,POP}_DISABLE_INTEGRAL_CONSTANT_OVERFLOW_WARNING
macros to avoid some MSVC weirdness.
The introduction of RotateLeft5() partly undoes one of the patches from bug
1443342, but that's unavoidable.
This change will help with static allocation of static atoms (bug 1411469).
MozReview-Commit-ID: 7r3PnrQXb29
VS 2017 15.6 (March 2018) doesn't seem to understand
`*DeclVal<SharedFontList*>()` anymore.
To work around this issue, the pointed-to type is now extracted in a separate
struct, for which we provide a specialization for raw pointers, so we don't
encounter the shaky `*DeclVal<T*>()` statement anymore.
MozReview-Commit-ID: FuslManbfdB
--HG--
extra : rebase_source : be3813aa9a028e6891cb3de1fc4faae5bde0348e
mozWritePoison secretly depended on the passed-in pointer being aligned
as though it were a pointer to uintptr_t, as it used bare stores to
C-casted pointers to accomplish poisoning. But this is an unnecessary
limitation: we can use memcpy and rely on the compiler to appropriately
inline the store to an unaligned store instruction if necessary.
The documentation for mozWritePoison says that only an even number of
sizeof(uintptr_t) bytes are overwritten; any trailing bytes are not
touched. This documentation doesn't correspond to how the function
actually works. The function as written will happily overwrite trailing
bytes and any bytes not contained in the object, if the passed-in size
isn't divisible by sizeof(uintptr_t). Let's fix that.
Nicholas Nethercote
Masatoshi Kimura
Margareta Eliza Balazs
Thomas Wisniewski
Gerald Squelart
Jeff Walden
Chris Peterson
Bryce Van Dyk
Randell Jesup
Brian Hackett
Nathan Froyd
shindli
Jeff Gilbert
Andreas Farre
Dylan Roeh
Cameron McCormack
Kris Maglione
Jacek Caban
Benjamin Peterson
Andi-Bogdan Postelnicu
Alessio Placitelli
André Bargull
Eric Rahm
arthur.iakab
Miko Mynttinen
Emilio Cobos Álvarez
Henri Sivonen
Daniel Stenberg
Paul Adenot
Csoregi Natalia
Alex Gaynor
Bogdan Tara
Gurzau Raul
Tom Ritter
Noemi Erli
Mike Hommey
Ryan VanderMeulen
Aaron Klotz
Jon Coppeard
Tom Bannister
Sebastian Hengst
Tristan Bourvon
Sylvestre Ledru
Jonathan Kew
Narcis Beleuzu
Dorel Luca
Jason Orendorff