add cmake magic to download google-benchmark, implement benchmark for #1337 #1381
Conversation
perf/range_conversion.cpp
Outdated
| auto palindrome_range_to(std::vector<std::string> const & words) | ||
| { | ||
| auto is_palindrome = [](auto const & word) { | ||
| return not ranges::empty(word) and ranges::equal(word, word | views::reverse); |
There was a problem hiding this comment.
It pains me to mention this, but you might like to grep for not, and, and or, since you did a massive overhal earlier in the year.
| // http://www.boost.org/LICENSE_1_0.txt) | ||
| // | ||
| // Project home: https://github.com/ericniebler/range-v3 | ||
| // |
There was a problem hiding this comment.
I'm cool with the code from #1337 being under the Boost licence, so long as it doesn't prevent me from also continuing to distribute it under Apache 2 with LLVM exception (IANAL but my understanding is that this is okay).
There was a problem hiding this comment.
BSL is "free for all uses, and you don't even need to include this license but you do have to include copyright." Speaking of which, this code is also yours so I'll add a copyright notice for you unless you say otherwise.
There was a problem hiding this comment.
SGTM (I use this code in classrooms).
| words_.push_back("his"); | ||
| words_.push_back("face"); | ||
| words_.push_back("abba"); | ||
| words_.push_back("toot"); |
There was a problem hiding this comment.
I read in a dictionary of words from file, but this will make things easier, and apparently still highlights the issue.
perf/range_conversion.cpp
Outdated
| for(auto _ : st) | ||
| { | ||
| auto words = ::palindrome_range_common(words_); | ||
| benchmark::DoNotOptimize(words.data()); |
There was a problem hiding this comment.
What is the difference between benchmark::DoNotOptimize(words) and benchmark::DoNotOptimize(words.data())? I did the former.
There was a problem hiding this comment.
I'm not sure, but I think the latter is necessary for ClobberMemory to have any meaningful effects (if I'm reading it right).
| { | ||
| auto words = ::palindrome_range_common(words_); | ||
| benchmark::DoNotOptimize(words.data()); | ||
| benchmark::ClobberMemory(); |
There was a problem hiding this comment.
I also forgot to clobber, but I suspect that would only make my times look better?
perf/range_conversion.cpp
Outdated
| for(auto _ : st) | ||
| { | ||
| auto words = ::palindrome_range_to(words_); | ||
| benchmark::DoNotOptimize(words_.data()); |
There was a problem hiding this comment.
| benchmark::DoNotOptimize(words_.data()); | |
| benchmark::DoNotOptimize(words.data()); |
There was a problem hiding this comment.
Alternatively, s/\<words\>/result/g.
There was a problem hiding this comment.
This is the only comment that triggered "Request changes".
No description provided.