[PATCH] D129533: [ADT] Pass string_view via copy

[ppenguin via Phabricator via llvm-commits]

prehistoric-penguin added a comment.

In D129533#3644282 <https://reviews.llvm.org/D129533#3644282>, @saugustine wrote:

> Why should these be different than the other constructors which don't take copies?

Thanks, take the cheap object `std::string_view` by copy is the preferred way:

- They have performance differences, pass by reference has one more level of indirection, so we need one more instruction when compared with the copy way

For such code snippet <https://godbolt.org/z/19rWfd96W>:

  cpp
  #include <string>
  #include <benchmark/benchmark.h>
  void TakesStringViewByValue(std::string_view s)
  {
      benchmark::DoNotOptimize(s.data());
  }
  void TakesStringViewByRef(const std::string_view& s)
  {
      benchmark::DoNotOptimize(s.data());
  }

We get the disassembly after build with `-O3`:

  asm
  TakesStringViewByValue(std::basic_string_view<char, std::char_traits<char> >): # @TakesStringViewByValue(std::basic_string_view<char, std::char_traits<char> >)
    movq %rsi, -8(%rsp)
    retq
  TakesStringViewByRef(std::basic_string_view<char, std::char_traits<char> > const&): # @TakesStringViewByRef(std::basic_string_view<char, std::char_traits<char> > const&)
    movq 8(%rdi), %rax
    movq %rax, -8(%rsp)
    retq
  _GLOBAL__sub_I_example.cpp: # @_GLOBAL__sub_I_example.cpp
    jmp benchmark::internal::InitializeStreams()@PLT # TAILCALL

And according to the abseil c++ tips week 1 <https://abseil.io/tips/1>, they also recommend us to pass `std::string_view` by copy:

> Unlike other string types, you should pass string_view by value just like you would an int or a double because string_view is a small value.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129533