On Thu, Aug 16, 2018 at 6:07 AM, Alex Gaynor <agay...@mozilla.com> wrote:
> Would it make sense to consider giving nsTHashtable and PLDHashTable > different names? Right now the names suggest that we have 3 general purpose > hash tables, but it might be easier if the names better suggested their > purpose, e.g. PLDHashTable -> MinimalCodeSizeHashTable (I'm sure we can do > better than that). > nsTHashTable is just a templated wrapper around PLDHashTable. For reference we also have nsDataHashtable, nsClassHashtable, nsRefPtrHashtable, nsInterfaceHashtable [1] all of which are rather convenient wrappers around PLDHashTable. These have also been implemented in such a way that the templates have lower code size overhead (thanks froydnj!). I somewhat prefer their interfaces to mfbt::HashMap (default infallible, plenty of predefined hashers, nice lookup for add semantics, etc), but to each their own. [1] https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Guide/Hashtables > Similarly, would it make sense to consolidate the APIs, as much as > possible, if the primary differences are around implementation details like > "how much is templated/inlined"? > > Really we just have PLDHashTable for historical reasons, not because of text size. I'm not sure we even want to keep PLDHashTable at all. We *have* found some issues where there's higher memory overhead due to the way our template wrappers pad their key structures and having a much lower level, but slightly annoying to use hashtable ended up being a good thing. Additionally we've been thinking about implementing a better hash algorithm such as round-robin hashing [2] , I'm not sure if that would work w/ mfbt::HashMap, but it would be nice to implement it in one place rather than two. [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1402910 -e > Alex > > > On Wed, Aug 15, 2018 at 5:39 AM Nicholas Nethercote < > n.netherc...@gmail.com> > wrote: > > > Hi, > > > > I recently moved Spidermonkey's js::Hash{Map,Set} classes from > > js/public/HashTable.h into mfbt/HashTable.h and renamed them as > > mozilla::Hash{Map,Set}. They can now be used throughout Gecko. > > > > (js/public/HashTable.h still exists in order to provide renamings of > > mozilla::Hash{Map,Set} as js::Hash{Map,Set}.) > > > > Why might you use mozilla::Hash{Map,Set} instead of PLDHashTable (or > > nsTHashtable and other descendants)? > > > > - Better API: these types provide proper HashMap and HashSet instances, > and > > (in my opinion) are easier to use. > > > > - Speed: the microbenchmark in xpcom/rust/gtest/bench- > collections/Bench.cpp > > shows that mozilla::HashSet is 2x or more faster than PLDHashTable. E.g. > > compare "MozHash" against "PLDHash" in this graph: > > > > > > https://treeherder.mozilla.org/perf.html#/graphs? > timerange=604800&series=mozilla-central,1730159,1,6& > series=mozilla-central,1730162,1,6&series=mozilla- > central,1730164,1,6&series=mozilla-central,1732092,1,6& > series=mozilla-central,1730163,1,6&series=mozilla-central,1730160,1,6 > > > > Bug 1477627 converted a hot hash table from PLDHashTable to > > mozilla::HashSet and appears to have sped up cycle collection in some > cases > > by 7%. If you know of another PLDHashTable that is hot, it might be worth > > converting it to mozilla::HashTable. > > > > Both mozilla::Hash{Map,Set} and PLDHashTable use the same double-hashing > > algorithm; the speed difference is due to mozilla::HashSet's extensive > use > > of templating and inlining. The downside of this is that mozilla::HashSet > > will increase binary size more than PLDHashTable. > > > > There are overview comments at the top of mfbt/HashTable.h, and the > classes > > themselves have more detailed comments about every method. > > > > Nick > > _______________________________________________ > > dev-platform mailing list > > dev-platform@lists.mozilla.org > > https://lists.mozilla.org/listinfo/dev-platform > > > _______________________________________________ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
