search

boostorg / container_hash

Generic hash function for STL style unordered containers
https://boost.org/libs/container_hash
32 stars 38 forks source link

Add support for std::coroutine_handle #32

Open cmazakas opened 1 year ago

cmazakas commented 1 year ago

We need to keep with the times

https://godbolt.org/z/1andznGhq

pdimov commented 1 year ago

What's the intended use of boost::hash<coroutine_handle>?

cmazakas commented 1 year ago

What's the intended use of boost::hash<coroutine_handle>?

In my case, it's for storing an unordered_flat_set<coroutine_handle> for use as an io_uring runtime. To this end, I can store and lookup tasks as required.

The other motivation is just a simple: "Well, the stdlib does it so we should too"

pdimov commented 1 year ago

Aren't you going to store tasks in the unordered set, though, instead of the raw coroutine_handle? In which case you'll need to specialize boost::hash<task> anyway.

cmazakas commented 1 year ago

Aren't you going to store tasks in the unordered set, though, instead of the raw coroutine_handle? In which case you'll need to specialize boost::hash<task> anyway.

This is a good question and I don't necessarily have a great response to it.

I might wind up storing coroutine_handle<> which is type-erased on the promise_type. This way, the runtime can store arbitrary tasks so long as everything it needs is kept in the promise type or coroutine frame itself. But I'm hesitant to push for this as I have no idea at this stage if this is viable or not.

I can, instead, do some external market research and see how other people are using hash<coroutine_handle<T>>.

pdimov commented 1 year ago

The test is overspecified; it should test that the hashes of t and t2 match and that they don't match the hash of t3 (a second task).

We don't want to guarantee that hashing a coroutine_handle produces the same result as hashing .address, because we want to be able to return the result of std::hash<std::coroutine_handle>.

If we want to be stricter than that and check that the hash isn't of extremely poor quality, we can do a collision test with something like 256 values, checking that there are no duplicates, as is done f.ex. in https://github.com/boostorg/container_hash/blob/develop/test/hash_number_test2.cpp.

pdimov commented 1 year ago

On the other hand, for transparent hashing purposes, we do want to guarantee that boost::hash<std::coroutine_handle>()( h ) == boost::hash<void*>()( h.address() ), and if std::hash<std::coroutine_handle> is used, this won't hold. :-/