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

[Samuel Benzaquen via Phabricator]

sbenza added a comment.

>> It seems like the input/benchmark generation in algorithms.bench is not a good fit for binary search.
>> 
>> Also, please take a look: https://github.com/llvm-mirror/libcxx/blob/master/benchmarks/algorithms.bench.cpp#L45
>>  I'm not sure if it matters, but seems like a weird idea to measure on 0...n - not very representative data.
>>  I would also suggest rewriting it like:
>> 
>>   V.resize(N);
>>   std::iota(V.begin(), V.end());
>> 
>> 
>> The last time I measured - much faster (and less code).
> 
> @sbenza wrote those benchmarks, there might be a reason why he used 0...n.

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.


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

https://reviews.llvm.org/D53994