--------------------------------------------------------------- Benchmark old new --------------------------------------------------------------- bm_vector_bool_count/1 1.92 ns 1.92 ns bm_vector_bool_count/2 1.92 ns 1.92 ns bm_vector_bool_count/3 1.92 ns 1.92 ns bm_vector_bool_count/4 1.92 ns 1.92 ns bm_vector_bool_count/5 1.92 ns 1.92 ns bm_vector_bool_count/6 1.92 ns 1.92 ns bm_vector_bool_count/7 1.92 ns 1.92 ns bm_vector_bool_count/8 1.92 ns 1.92 ns bm_vector_bool_count/16 1.92 ns 1.92 ns bm_vector_bool_count/64 2.24 ns 2.25 ns bm_vector_bool_count/512 3.19 ns 3.20 ns bm_vector_bool_count/4096 14.1 ns 12.3 ns bm_vector_bool_count/32768 84.0 ns 83.6 ns bm_vector_bool_count/262144 664 ns 661 ns bm_vector_bool_count/1048576 2623 ns 2628 ns bm_vector_bool_ranges_count/1 1.07 ns 1.92 ns bm_vector_bool_ranges_count/2 1.65 ns 1.92 ns bm_vector_bool_ranges_count/3 2.27 ns 1.92 ns bm_vector_bool_ranges_count/4 2.68 ns 1.92 ns bm_vector_bool_ranges_count/5 3.33 ns 1.92 ns bm_vector_bool_ranges_count/6 3.99 ns 1.92 ns bm_vector_bool_ranges_count/7 4.67 ns 1.92 ns bm_vector_bool_ranges_count/8 5.19 ns 1.92 ns bm_vector_bool_ranges_count/16 11.1 ns 1.92 ns bm_vector_bool_ranges_count/64 52.2 ns 2.24 ns bm_vector_bool_ranges_count/512 452 ns 3.20 ns bm_vector_bool_ranges_count/4096 3577 ns 12.1 ns bm_vector_bool_ranges_count/32768 28725 ns 83.7 ns bm_vector_bool_ranges_count/262144 229676 ns 662 ns bm_vector_bool_ranges_count/1048576 905574 ns 2625 ns
Details
- Reviewers
ldionne - Group Reviewers
Restricted Project - Commits
- rGa9138cdb36cc: [libc++] Optimize ranges::count for __bit_iterators
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
libcxx/include/__algorithm/count.h | ||
---|---|---|
35 | We normally name these algorithms just __count. | |
76 | Can you make sure that these code paths are tested? We could have a test like libcxx/test/std/algorithms/alg.nonmodifying/alg.count/count.vector_bool.pass.cpp *or* libcxx/test/std/containers/sequences/vector.bool/count.pass.cpp, but we should have something. I would recommend breaking this logic in a very obvious way to first see whether we have an existing test for this. | |
libcxx/include/__algorithm/iterator_operations.h | ||
176–177 ↗ | (On Diff #547055) | I think I would avoid introducing this, I don't think it provides that much value. |
LGTM with comments applied and CI passing.
libcxx/include/__algorithm/count.h | ||
---|---|---|
46 | And we should make sure that we have a test for that, i.e. call std::count(bit-iterator) and ranges::count(bit-iterator) in constexpr in C++20 mode. | |
libcxx/include/__algorithm/iterator_operations.h | ||
176–177 ↗ | (On Diff #547055) | ^ I wouldn't necessarily object to introducing these types, but we should do that separately and handle not only __difference_type. |
libcxx/test/std/algorithms/alg.nonmodifying/alg.count/count.pass.cpp | ||
---|---|---|
1 | We need a test with ranges::count and std::vector<bool> too. |
We normally name these algorithms just __count.