This is an archive of the discontinued LLVM Phabricator instance.

Modifications to the algorithm sort benchmark
ClosedPublic

Authored by minjaehwang on Jun 12 2020, 3:32 PM.

Details

Reviewers
EricWF
mvels
Group Reviewers
Restricted Project
Commits
rG842136428264: Modifications to the algorithm sort benchmark
Summary

Modifies the algorithm sort bench:

  • shows sorting time per element, instead of sorting time per array.

This would make comparison between different sizes of arrays easier.

  • adds std::pair benchmark cases.
  • uses a large number of arrays to benchmark, instead of repeatedly sorting the same array.
  • sorting the same array again and again would not show actual sorting performance over randomized data sets.

Diff Detail

Event Timeline

minjaehwang created this revision.Jun 12 2020, 3:32 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 12 2020, 3:32 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
EricWF accepted this revision.Jun 17 2020, 1:36 PM

LGTM

libcxx/benchmarks/algorithms.bench.cpp
222

Could you make this boolean value an enum, so it's clear what it means?

This revision is now accepted and ready to land.Jun 17 2020, 1:36 PM

Reflects EricWF's comment. Change a bool variable to an enum.

Fix clang-tidy warnings.

clang-tidy complains that "benchmark/benchmark.h" is not found at CartesianBehcmarks.h. This diff does not modify that file and this is a benign error. It compiles without a problem.

Could anyone land this diff into the master?

mvels added a reviewer: mvels.Jul 6 2020, 1:01 PM

clang-tidy complains that "benchmark/benchmark.h" is not found at CartesianBehcmarks.h. This diff does not modify that file and this is a benign error. It compiles without a problem.

Could anyone land this diff into the master?

I'll take a stab at committing this diff

mvels accepted this revision.Jul 6 2020, 1:02 PM
mvels retitled this revision from Modifies the algorithm sort bench. - shows sorting time per element, instead of sorting time per array. This would make comparison between different sizes of arrays easier. - adds std::pair benchmark cases. - uses a large number of arrays to... to Modifications to the algorithm sort benchmark .Jul 6 2020, 1:05 PM
mvels edited the summary of this revision. (Show Details)
mvels added inline comments.Jul 6 2020, 1:09 PM
libcxx/benchmarks/algorithms.bench.cpp
174

s/bencvhmark/benchmark/

mvels requested changes to this revision.Jul 6 2020, 1:11 PM
This revision now requires changes to proceed.Jul 6 2020, 1:11 PM
minjaehwang retitled this revision from Modifications to the algorithm sort benchmark to Modifications to the algorithm sort benchmark.

Fix the typo

minjaehwang marked 2 inline comments as done.Jul 6 2020, 1:49 PM

Resolved your comments!

mvels accepted this revision.Jul 6 2020, 3:27 PM
This revision is now accepted and ready to land.Jul 6 2020, 3:27 PM
This revision was automatically updated to reflect the committed changes.