Skip to content

Commit

Permalink
Remove unnecessary copies in dynamic::hash()
Browse files Browse the repository at this point in the history
Summary:
`dynamic::hash()` would copy every key-value pair in the object accumulator hash because of two bugs in the code:
1. The lambda took `auto` instead of `auto const&`
2. The hasher was `hash<pair<dynamic, dynamic>>` not `hash<pair<dynamic const, dynamic>>` meaning a conversion was needed.

These bugs together caused 2 copies for each sub-object. Since the copies are recursive, each object got copied 2*depth times.

Reviewed By: yfeldblum

Differential Revision: D16452213

fbshipit-source-id: 64a55e1640abb022c148183646e9f9720fd8482e
  • Loading branch information
terrelln authored and facebook-github-bot committed Jul 24, 2019
1 parent 0752c05 commit 6b849e9
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions folly/dynamic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -292,12 +292,12 @@ std::size_t dynamic::hash() const {
// Accumulate using addition instead of using hash_range (as in the ARRAY
// case), as we need a commutative hash operation since unordered_map's
// iteration order is unspecified.
auto h = std::hash<std::pair<dynamic, dynamic>>{};
auto h = std::hash<std::pair<dynamic const, dynamic>>{};
return std::accumulate(
items().begin(),
items().end(),
size_t{0x0B1EC7},
[&](auto acc, auto item) { return acc + h(item); });
[&](auto acc, auto const& item) { return acc + h(item); });
}
case ARRAY:
return folly::hash::hash_range(begin(), end());
Expand Down

0 comments on commit 6b849e9

Please sign in to comment.