Re: [boost] Add an adapter to cover fast bit functions

Subject: Re: [boost] Add an adapter to cover fast bit functions
From: Evgeny Shulgin [Izaron] (izaronplatz_at_[hidden])
Date: 2018-08-27 23:15:03

Thank you for your reply! It really helped me a lot.

> - Please don't #include <iostream> in headers, if you can avoid it.
My bad, I forgot to remove this huge unused header, I should've used a tool
like include-what-you-use.

> - You're using C++14 constexpr, so BOOST_CXX14_CONSTEXPR
>   would be the correct macro.
Thanks, I missed the fact that in C++11 it is correct only when the function
has exactly one return

> - You might need to think about whether/how to support
>  signed integers. In particular, I'm thinking about
>  swap and rotate.
I guess "rotate" should not affect the sign bit. If I remember correctly,
there are commands in Assembler to rotate signed numbers.
"Swap" works with whole bytes and will implicitly take a number as an
unsigned. But it's a theme to discuss, as for other functions.

It's good to check signed types in the benchmark as well.

> - For unit tests, it's better to use a prng instead of random_device.
>   non-deterministic tests make it more difficult to reproduce
>   test failures.
I'll change it, sounds good.

> - It's better to separate benchmarks from unit tests. In particular,
>   storing the elements in a vector is necessary for benchmarking,
>   since you don't want to benchmark the generation of the
>   values, but for tests, it artificially limits the maximum
>   number of values you can check.
Values generation isn't benchmarked, only working time of functions.

I was thinking of separating the whole stuff in this way:
- a single unit test verifies the correctness of the algorithm comparing it
with naive functions on generated values
- the benchmark file isn't a test really, but could be used in CI to produce
information about performance
  if it's possible (though I guess it's be possible)

Also, IMO, there should be a tiny single header for _every_ function, since
a large file with a ton of monotonic specializations
is pretty hard to support.

> - For char and short, testing every possible input is easy.
>   You can do int, too, if you're willing to wait a bit for
>   it to finish.
I think it's too much to check every int32 value, because it takes really a
lot of time for almost nothing. We can use many generated values
with corner cases like the maximal/minimal value of each type. The idea is
fine.

> Or Boost.Integer.
I explored this, it looks more appropriate to insert fresh code. I hope
someone would take a look at PR if I make the code look
really much better and send it.

--
Sent from: http://boost.2283326.n4.nabble.com/Boost-Dev-f2600599.html