Skip to content

Conversation

@stgatilov
Copy link

See #73

stgatilov added 3 commits May 11, 2024 14:27
…unction on uint64_t.

It constructs a set of distinct keys with zero lower half, then computes hash function for all of them, and checks that the hashes are mostly different in lower bits.
@Tessil
Copy link
Owner

Tessil commented May 26, 2024

Thanks for the contribution.

The change looks good but I'm a bit worried about the backward compatibility and the risk of changing the performance of existing users. We're applying the finalizer by default no matter the type of T, the std lib or the GrowthPolicy.

I wonder if we should instead provide the tsl::hash as utility but keep the default class Hash = std::hash<Key> and put a note in the README.

@stgatilov
Copy link
Author

Finalizer is not applied on MSVC, i.e. it is only applied on libstd-c++ and libc++.
It is maybe OK to ignore it with prime size policy: I can add this tweak if you want.

In my opinion, if you don't enable finalizer by default and keep default size policy as "power of two", then it is incorrect to say that tsl map is drop-in replacement for std::unordered_map (or other hash map implementations) and that no expertise is required to use it. And you should certainly static_assert against using pointers as keys then, because this is the case where this issue almost always happens.

If you have some specific benchmarks in mind, I can run them.
There should be little difference on large hash tables (since they are memory-limited).
And if the results of some benchmark changes a lot, then I would argue that the previous results are a lie in a sense. Reliably spreading elements across table to provide O(1) average time complexity is the main feature of hash table. It is not fair to compare an implementation which does not have this feature with implementations which do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants