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

Tue Dec 4 14:16:29 PST 2018

ldionne added a subscriber: sbenza.
ldionne added a comment.

In D53994#1319173 <https://reviews.llvm.org/D53994#1319173>, @dyaroshev wrote:

> In D53994#1319017 <https://reviews.llvm.org/D53994#1319017>, @ldionne wrote:
>
> > @dyaroshev Can you please rebase on top of master? There's been significant changes in algorithms.bench.cpp and the patch does not apply cleanly anymore.
>
>
> It seems like I need to redo my benchmarks from scratch to match what is now used. It will take me a couple of hours, so probably on the weekend.

Ugh, sorry about that.

> Do you mind if I create a separate file: "algorithms.search.bench" and rename this one in "algorithms.sort.bench"?

No, I think that's a fine idea.

> 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.

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

https://reviews.llvm.org/D53994