[libcxx-commits] [libcxx] Optimize input iterator overload of `std::vector::assign(first, last)` (PR #113852)

[Peng Liu via libcxx-commits]

================
@@ -48,6 +48,76 @@ void BM_Assignment(benchmark::State& st, Container) {
   }
 }
 
+// Wrap any Iterator into an input iterator
+template <typename Iterator>
+class InputIterator {
+  using iter_traits = std::iterator_traits<Iterator>;
+
+public:
+  using iterator_category = std::input_iterator_tag;
+  using value_type        = typename iter_traits::value_type;
+  using difference_type   = typename iter_traits::difference_type;
+  using pointer           = typename iter_traits::pointer;
+  using reference         = typename iter_traits::reference;
+
+  InputIterator(Iterator it) : current_(it) {}
+
+  reference operator*() { return *current_; }
+  InputIterator& operator++() {
+    ++current_;
+    return *this;
+  }
+  InputIterator operator++(int) {
+    InputIterator tmp = *this;
+    ++(*this);
+    return tmp;
+  }
+
+  friend bool operator==(const InputIterator& lhs, const InputIterator& rhs) { return lhs.current_ == rhs.current_; }
+  friend bool operator!=(const InputIterator& lhs, const InputIterator& rhs) { return !(lhs == rhs); }
+
+private:
+  Iterator current_;
+};
+
+template <typename Iterator>
+InputIterator<Iterator> make_input_iterator(Iterator it) {
+  return InputIterator<Iterator>(it);
+}
+
+template <class Container,
+          class GenInputs,
+          typename std::enable_if<std::is_trivial<typename Container::value_type>::value>::type* = nullptr>
+void BM_AssignInputIterIter(benchmark::State& st, Container c, GenInputs gen) {
+  auto in = gen(st.range(1));
+  benchmark::DoNotOptimize(&in);
+  for (auto _ : st) {
+    st.PauseTiming();
+    c.resize(st.range(0));
+    benchmark::DoNotOptimize(&c);
+    st.ResumeTiming();
----------------
winner245 wrote:

Thank you for your feedback.

In my initial commit, I included `resize()` in the loop. However, since I am measuring the performance of `assign()`, any other operations, including `resize()`, inside the loop constitute noise to my measurements. That is why I originally used `PauseTiming()` and `ResumeTiming()` before and after the `resize()` call, as shown below:

```cpp
st.PauseTiming();
c.resize(st.range(0));
benchmark::DoNotOptimize(&c);
st.ResumeTiming();
```

In your previous review, you mentioned:

> PauseTiming and ResumeTiming shouldn't be used in a single-iteration benchmark whenever possible.

To address this comment, I removed the `PauseTiming()` and `ResumeTiming()` call from the loop. Consequently, I had to move `resize` out of the loop because my purpose is to measure `assign` only. 

However, in your current view, you stated 

> The resize() should be in the loop to ensure the vector doesn't grow forever 

This seems contradictory, and I am quite confused. If I were to put `resize()` back in the loop, I would also have to call `PauseTiming()` and `ResumeTiming()` around `resize()`, which would revert to my first commit. 

In my opinion, calling `resize()` once before entering the loop is sufficient. The `resize()` call just ensures that I start with a non-empty vector, and inside the loop, the assign function itself takes care of memory reallocation as needed. I believe there is no risk of the vector growing indefinitely in this context.

I appreciate your insights and would be happy to clarify this point further if needed.

https://github.com/llvm/llvm-project/pull/113852