[PATCH] D53994: Fixing lower bound regression in certain situations.

[Denis Yaroshevskiy via Phabricator]

dyaroshev added a comment.

In D53994#1319307 <https://reviews.llvm.org/D53994#1319307>, @sbenza wrote:

> re resize+iota, I don't see why not. That code is not part of the measurements.
>
> As for why `0...n`, for integers it doesn't really what the values are. only that there are N distinct values. The initial ordering of those values is changed depending on the benchmark.
>  Note that for strings we don't use consecutive values as the value itself matters for `operator<`. That is, the first mismatched char is the one that matters and "sequential" strings would not be good for that.
>
> For `lower_bound` you might want a different sequence, though. You might also want to check for values missing in the list. The way we do it for std::set is to have `0,2,...,N`. We then search for `2*i` for hits and `2*i+1` for misses.


I see your point about n values but I'm not entirely sold.
There is a famous bug in quick sort implementation, for example, - not partitioning the duplicates.
BTW just ran a small benchmark, check out how much duplicates affect the performance of std::sort http://quick-bench.com/mq8UiBPBT5vmmS7FG-RmY11VriQ

I also suspect that median of 5 type of algorithm and other similar "find value to partition" techniques could  depend on the distribution - but maybe not.

Anyways, this is minor - just was confused a bit.

Why do you think for lower bound it is beneficial to look for "not found elements"? It never checks for equality (it should just call a partition_point but it gets ugly().
I can run these benchmarks but I'd suggest not to commit them if they don't show anything.

Do you have any other suggestions? Popular sizes, 32/64 bit integers? I usually just pick 1000 and smth big but I don't know, maybe there is a research.

PS:
For strings I would suggest maybe to also use URL;s of some kind.
Random string comparisons results in a first character, right? But for URLs you often hit a long common prefix.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53994/new/

https://reviews.llvm.org/D53994