[libcxx-commits] [PATCH] D124740: [libc++] Granularize algorithm benchmarks

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

In general looks like an improvement, but I have some questions and remarks.



================
Comment at: libcxx/benchmarks/CMakeLists.txt:191
+    variant_visit_3.bench.cpp
+    vector_operations.bench.cpp
+    )
----------------
nice no more globbing!


================
Comment at: libcxx/benchmarks/CMakeLists.txt:213
   endif()
-  add_benchmark_test(${test_name} ${test_file})
+  add_benchmark_test(${test_name} ${test_path})
 endforeach()
----------------
Can you explain why this is done?


================
Comment at: libcxx/benchmarks/algorithms/Common.h:1
+//===----------------------------------------------------------------------===//
+//
----------------
Can you use `common.h` instead of `Common.h`?


================
Comment at: libcxx/benchmarks/algorithms/Common.h:62
   V.resize(N);
-  for (int i = 0; i < N; ++i) {
+  for (unsigned int i = 0; i < N; ++i) {
     V[i] = gas;
----------------
Please try to do these changes in a separate commit and not while moving code.
If this is the only change, I won't object.


================
Comment at: libcxx/benchmarks/algorithms/make_heap.bench.cpp:29
+};
+} // namespace
+
----------------
Does an anonymous namespace add value here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124740