Re: Formal review request for Hash2

From: Ivan Matek (libbooze_at_[hidden])
Date: 2024-12-04 19:55:32

On Wed, Dec 4, 2024 at 8:38 PM Peter Dimov <pdimov_at_[hidden]> wrote:

> Ivan Matek wrote:
> > 2.    exploit that before padding we are contiguous and trivial so pass
> that
> > region in one call to hash_append_range
>
> Note by the way that `double` is not contiguously hashable because +0.0 is
> equal to -0.0, but their binary representations differ.
>
> This is a known trap and N3980 mentions it:
>
>
> https://open-std.org/jtc1/sc22/wg21/docs/papers/2014/n3980.html#is_contiguously_hashable
> #
>

Yes, my mistake, and I even read the abseil::Hash documentation so that is
3 places that mention it(proposal, hash2 docs, abseil docs), but situation
might still happen, if for example all those members were integers.

Would in theory would it be possible to detect those regions with Describe
and fuse the append calls?
I ask because my original idea was to just allow users to pass pointers to
members, e.g.   ..., this, &MyStruct::x, &MyStruct::y, &MyStruct::z) before
I learned offsetof is quite ugly, i.e. there is no way to go from member
ptr to offset of member. I checked Describe docs and offset is not
mentioned so I presume that information is unavailable?
Not a big deal, as I mentioned difference between 2. and 1. approach is not
large.
Also thank you for the clang inlining hint, adding inline-threshod removes
the difference in perf between 1. and 3, so I can confirm that now for me
on my machine manually written memberwise hashing is as fast as automagic
Describe one. Very impressive and nice use of existing Boost component.
For curious this is how now my CMake section looks.

if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -stdlib=libc++ -mllvm
-inline-threshold=1000")
endif ()