[libcxx-commits] [PATCH] D132505: [libc++] Refactor deque::iterator algorithm optimizations

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/benchmarks/deque_iterator.bench.cpp:124
+  auto operator()(Args... args) const {
+    std::move(std::forward<Args>(args)...);
+  }
----------------
EricWF wrote:
> Can you meaningfully do this more than once on the same container?
When the types are trivial they are just copied, so it should be fine.


================
Comment at: libcxx/benchmarks/deque_iterator.bench.cpp:1
+//===----------------------------------------------------------------------===//
+//
----------------
EricWF wrote:
> philnik wrote:
> > ldionne wrote:
> > > I would be curious to try adding `_LIBCPP_ALWAYS_INLINE` to `__unwrap_and_dispatch` (and possibly other implementation details in that code path) to see what impact it has on the benchmarks. Could you try that out and report? I would assume that for raw pointers, that should all end up being inlined away.
> > I've changed the implementation of `copy` and `move` to use the segment iterators and that fixed the performance issues, so I didn't test with `_LIBCPP_ALWAYS_INLINE`. 
> Have you verified the assembly for these benchmarks?
No, but why should I? The assembly is obviously different, since the benchmarks show differences. The differences aren't very significant though, i.e. a pessimization for the 0-case, but an optimization for small N for some algorithms and an optimization across the board for others.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132505