Yes, this is a cleanup effort I started on recently, having noticed the guidance from the Chromium Containers guide.
For instances of std::unordered_set (and instances of std::set that don't require iteration order), I've been replacing with the following priority:
- If the value is an enum, use base::EnumSet
- If all values can be determined at compile time, use base::fixed_flat_set
- If values are written once and the set is used only for lookups, use base::flat_set
- Use absl::flat_hash_set, unless pointer stability is required.
- If pointer stability is required for the value, use absl::flat_hash_set<key, std::unique_ptr<value>>, or absl::node_hash_set<key, value>.
Some discussion:
I noticed that absl::flat_hash_map/set usually increases binary/APK size over the STL container of the equivalent type. I've started mostly with migrating sets first, and one type at a time (std::string, int) so that reduces the growth, but I notice this more on swapping maps, as swapping out a std::unordered_map with absl::flat_hash_map of the same type generally adds 0.5-1.5KiB to the APK size. There only a few hundred instances of std::unordered_map, so a total migration would result in less than 1 MiB APK size increase, but an FYI.
Updating headers to add the Abseil includes on widely-used headers can result in compile size increases that fail the CQ checks, for example in CLs like:
https://crrev.com/c/7542821 and
https://crrev.com/c/7546326. Using forward declarations to avoid adding includes in headers is tricky because of the optional template params and also needing to forward declare additional classes like the hasher and allocator. In cases like this, should I hold off on sending these CLs or just add a Gerritt footer to ignore the check as an unavoidable cost?