Re: [hash2] Formal Review Begins

From: Alan de Freitas (alandefreitas_at_[hidden])
Date: 2024-12-15 19:24:53

Here's my review.

I want to thank Peter Dimov and Christian Mazakas for their work.

> Does this library bring real benefit to C++ developers for real world
use-case?

Yes. Improving the state of hash functions for unordered containers is a
worthy goal, as is easily changing the hash function being used.

Choosing a specific hash algorithm or plugin external hash algorithms is
very useful.

I like the choice of hash algorithms. The list can be expanded if people
clamor for a new algorithm.

> Do you have an application for this library?

Yes. We always use unordered containers, which is the primary use case here.

> Does the API match with current best practices?

Yes. It's a significant improvement over "Boost.Hash1". Maybe it could just
be "Boost.Hash" since there's no "Boost.Hash1" and this library is not only
about containers.

I was initially concerned that `result()` was not idempotent, but Peter's
arguments made sense to me. The authors could change the name to some verb,
but that's up to them.

It is the same with the provided hashers being as cryptographically safe as
they could be, but that seems like an unfair criterion for a library
focused on containers and aiming to be an improvement over "Boost.Hash1".

However, in both cases, expectations have changed over time. Big
admonitions in the documentation are worth it.

hash_append_sized_range could become something like
hash_append_range_and_size, to avoid confusion with
std::ranges::sized_range.

> Is the documentation helpful and clear?

Yes. The documentation is very complete and precise. It's driven by goals
and examples, which is excellent.

Since the implementations are not supposed to be the fastest, the
documentation could include some benchmarks.

This could be the first example since unordered containers are the most
common use case. Then some content about wrapping external hashing
algorithms could be included, since the library is about generalizing that.

I wouldn't say I like single-page documentation very much, but it does the
job.

> Did you try to use it? What problems or surprises did you encounter?

Yes. I compiled the code and ran the examples.

> What is your evaluation of the implementation?

The code is very well organized and adequately documented.

I hope the authors can figure out an implementation for
hash_append_unordered_range that doesn't lose entropy with
cryptographically safe hash functions. Or maybe they could provide
alternatives and document their trade-offs. Or there could be some
hash_aggregator concept to let it be customized.

> Are you knowledgeable about the problem domain?

Yes. I have been using various hash functions in multiple applications for
a long time.

> Please explicitly state that you either *accept* or *reject* the
inclusion of this library into boost.

I recommend accepting the library.

> Also please indicate the time & effort spent on the evaluation and give
the reasons for your decision.

I read the documentation and the source code. I compiled the library and
ran the examples.

I spent time on and off over the last week evaluating the library. In
total, I spent about two days evaluating it.

> Disclaimer

I should also mention my affiliation with the C++ Alliance as a Staff
Engineer.

Em dom., 15 de dez. de 2024 Ã s 14:15, Matt Borland via Boost <
boost_at_[hidden]> escreveu:

> > Dear all,
> >
>
> > The review of Hash2 by Peter Dimov and Christian Mazakas begins today
> Saturday, December 7th through Sunday December 15th, 2024.
> >
>
> > Code: https://github.com/pdimov/hash2
> > Docs: https://pdimov.github.io/hash2/
> >
>
> > Please provide feedback on the following general topics:
> >
>
> > - What is your evaluation of the design?
> > - What is your evaluation of the implementation?
> > - What is your evaluation of the documentation?
> > - What is your evaluation of the potential usefulness
> > of the library? Do you already use it in industry?
> > - Did you try to use the library? With which compiler(s)? Did
> > you have any problems?
> > - How much effort did you put into your evaluation?
> > A glance? A quick reading? In-depth study?
> > - Are you knowledgeable about the problem domain?
> >
>
> > Ensure to explicitly include with your review: ACCEPT, REJECT, or
> CONDITIONAL ACCEPT (with acceptance conditions).
> >
>
> > There's already been a flurry of conversation so I thank you all for
> that. If you have any questions during this process feel free to reach out
> to me. Additionally, if you would like to submit your review privately,
> which I will anonymize for the review report, you may send it directly to
> me at: matt_at_mattborland.com.
> >
>
> > Matt Borland
> > Review Manager and C++Alliance Staff Engineer
>
> All,
>
> This is the final reminder that the review period for Hash2 ends tonight.
>
> Matt
> _______________________________________________
> Unsubscribe & other changes:
> http://lists.boost.org/mailman/listinfo.cgi/boost
>

-- 
Alan Freitas
https://alandefreitas.github.io/alandefreitas/
<https://github.com/alandefreitas>