[PATCH] D53994: Fixing lower bound regression in certain situations.
Samuel Benzaquen via Phabricator
reviews at reviews.llvm.org
Tue Dec 4 14:35:08 PST 2018
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:
>> 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
More information about the libcxx-commits