WIP util: Add absl's new flat_hash_map and flat_hash_set to the build system and use it in a couple of places

[jmarantz]

Description: absl::flat_hash_map is a newly open-sourced hash-table from Google that has very good memory and performance with certain hash-table patterns. This adds the new build infrastructure needed to bring in flat_hash_map/set and uses them in places where I could quickly measure memory-usage reduction.

For more info on what makes absl::flat_hash_map efficient, see https://www.youtube.com/watch?v=ncHmEUmJZf4&t=2371s (1 hour). TL;DR:

• flat_hash_(set_map) and node_hash_(set|map) are sweeping the google codebase as they are mostly better in every dimension to alternatives, particularly unordered_(set_map). Among the sources of hotness are aggressive exploitation of SSE2 instructions, and a very L1-cache-aware design.
• node_hash_(set|map) is better in every dimension most of the time, and is an easy-choice for a large-scale refactor. It's a 99.9% drop-in replacement with the rare exception being code that directly cares about bucket_count. This PR changes the maps from string in thread_local_store from unordered_map to node_hash_map.
• flat_hash_(set|map) is the best choice when the keys and values are small and cheap to move, as all of those are stored inline in the hash array -- there's no indirection. This means that when the table capacity is larger than its size, there will be more wasted space. This PR brings in flat_hash_(set|map) for some tables in the stats infrastructure that meet this criteria, and validates that with a memory-consumption unit test.

Note that some of the other PRs in flight to address #4196 will make flat_hash_map a win for more cases, notably #4711.

It is very hard to understand the benefit without measuring memory usage -- at a minimum, so this #4714 should be checked in first, and this should be a pattern for switching to this library.

Risk Level: medium -- brings in a new version of absl
Testing: //test/...
Docs Changes: n/a
Release Notes: n/a

[stale]

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

[jmarantz]

Ping

…

On Sat, Oct 20, 2018, 8:08 PM stale[bot] ***@***.*** wrote: This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#4715 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB2kPbRl4SiARAfXppXEyj9f0azE-5bNks5um7r_gaJpZM4Xax_S> .

[mattklein123]

I will review. Looks like it needs a master merge though.

[jmarantz]

I will probably be able to do a master merge on Wednesday.... Don't have access to my computer now. I was pinging just to avoid stalebot. The benchmark PR would be good to get in first anyway.

…

On Sat, Oct 20, 2018, 10:38 PM Matt Klein ***@***.*** wrote: I will review. Looks like it needs a master merge though. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#4715 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB2kPTnhCgwAk3OLv3QbVmXNPImUynBkks5um940gaJpZM4Xax_S> .

[mattklein123]

@jmarantz is this the next one? Merge master?

Also, I will watch the video, but do you mind briefly summarizing what a flat hash map is in the PR and why you think it's going to be better here? I'm just curious if flat hash map is always better or if it's not always better/

[jmarantz]

will do, the outstanding PRs can now be done in any order but I had turned my attention to the one where we don't copy strings into the map keys; about to remove the WIP status for that.

[mattklein123]

OK just ping me on whichever one you want me to review next when ready.

[jmarantz]

Leaving this a WIP as it probably would be nice to establish speed-test baseline as well. Maybe I'll switch the other thread-local-storage maps to node_hash_* so they are fast too, if not small. Once #4711 is in they can all be flat_hash_*

[jmarantz]

Bringing in the node_hash_* was easy and an immediate win, though less of a win than #4711 which is why I didn't pursue it before. But considering this PR independently it's easy enough.

I still want to add a baseline speed test though so we can see the perf impact, so leaving this a WIP

[jmarantz]

closing this for now as well in favor of #4872 , although this PR appears to be a modest unambiguous win even on its own. LMK if you'd prefer to get this in separately (e.g. so others could use flat_hash_map) and I can fix the CI issues (order-sensitivity in tests iterating over unordered-maps)