[libcxx-commits] [PATCH] D124122: [libc++] Optimize std::rotate

Tue May 17 11:16:33 PDT 2022

ldionne added a comment.

In D124122#3475497 <https://reviews.llvm.org/D124122#3475497>, @philnik wrote:

> In D124122#3475473 <https://reviews.llvm.org/D124122#3475473>, @ldionne wrote:
>
>> I'd be fine with this since it addresses the underwhelming performance for simple types like `int`, which is super important. However, I would prefer if we instead went for a better algorithm directly, like the swap/grail rotate algorithm mentioned in https://github.com/llvm/llvm-project/issues/54949#issue-1206295098.
>
> I planned to do a follow-up for that anyways. Would it be OK if I just nuke the current implementation and add the `ranges` API in the same patch?

Yes, if we can have perf benchmarks for before and after. If that's easy to do, you could also first nuke the current implementation and replace it by one that is `ranges::`-friendly, and then just add the tiny `ranges::rotate` overlay on top (+ tests) in a separate patch. Both ways are acceptable, though.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124122