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

Denis Yaroshevskiy via Phabricator reviews at reviews.llvm.org
Tue Dec 4 13:18:17 PST 2018


dyaroshev added a comment.

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.
Do you mind if I create a separate file: "algorithms.search.bench" and rename this one in "algorithms.sort.bench"?
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).


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

https://reviews.llvm.org/D53994





More information about the libcxx-commits mailing list