greg7mdp / parallel-hashmap

A family of header-only, very fast and memory-friendly hashmap and btree containers.
https://greg7mdp.github.io/parallel-hashmap/
Apache License 2.0
2.47k stars 234 forks source link

Iteration Order Differs on Arm64 Architecture #212

Closed fegennari closed 10 months ago

fegennari commented 10 months ago

We recently added arm64 as a supported architecture and I've found that a number of tests are failing because the phmap iteration order differs on arm64 (compare to x86-64) on linux. The powerPC build iteration order agrees with x86-64. The differences are very minor, maybe a few percent of the elements are in a different order. This isn't a big deal, but I'm curious why there would be a difference. None of the other hash_maps I've tested show a difference across these architectures.

greg7mdp commented 10 months ago

Hum, I think that in general, tests should not depend on the order of iteration in any hash map. The iteration order is definitely not a provided guarantee, and indeed some containers (like the original abseil) make specific efforts to randomize iteration order (from here: "While std::unordered_map makes no guarantees about iteration order, many implementations happen to have a deterministic order based on the keys and their insert order. This is not true of absl::flat_hash_map and absl::node_hash_map. Thus, converting from std::unordered_map to absl::flat_hash_map can expose latent bugs where the code incorrectly depends on iteration order.").

Out of curiosity, what is the key type and hash function you are using for these containers?

fegennari commented 10 months ago

It's not really a bug, I'm mostly curious why the order differs. The tests that fail are ones that write some hash map contents out to disk and compare with golden results. My workaround has been to canonically sort the results for both the golden and actual run, and then everything agrees. As far as I can tell, gcc's hash_map, unordered_map, and google::sparse_hash_map agree across architectures.

The failing tests are all ones that unique polygon clips and write out the results. The key is a set of polygons (XY vertices) and the hash used is CityHash from here: https://github.com/google/cityhash. The hashes themselves agree - I printed them out. Only the iteration order disagrees. And it's only some minor changes, such as an isolated key jumping a few dozen spots in the order.

greg7mdp commented 10 months ago

Hum, not sure. My best guess as to the difference would be that maybe it uses different versions of phmap_mix<8> in phmap_utils.h (mixing is done after the hash computation), although in that case I would expect most keys to hash differently. If __int128 is supported on both platforms, and always provides the same results, then maybe there is a difference between the GroupSse2Impl (used on i86_64) and the GroupPortableImpl (used on ARM).

fegennari commented 10 months ago

Okay, thank you for looking into this. I'll continue to work around this using a sort on hashval, since this is only a problem with testing. It sounds like iteration order isn't guaranteed, so this isn't a bug. You can close this issue.