I think this is unfair:

void StringPieceFindTestVal(const std::vector<base::StringPiece>& vector) {
  for (base::StringPiece piece : vector)
    printf("%zu\n", piece.find('x'));
}

You would never want to copy each element of the vector while iterating. You'd write this as:

void StringPieceFindTestVal(const std::vector<base::StringPiece>& vector) {
  for (const auto& piece : vector)
    printf("%zu\n", piece.find('x'));
}

This seems orthogonal to the question of passing a StringPiece. By value allows rvalue optimizations that const& does not.

Thanks for investigating this.

My personal intuition is that const auto& would be better when iterating. The rationale is that if we're iterating through a list, the StringPiece is already somewhere in memory, so we have to incur the cost of the dereference to get the actual StringPiece anyway. So it's a matter of dereferencing upfront to get the by value copy, or dereferencing later when we use the reference to the StringPiece.

I am curious to see if changing the internal helpers to pass args by value will change anything though...

#1:
> I think this is unfair ... You would never want to copy each element of the vector while iterating.

But that is the very question I am trying to answer: should you iterate by value or by reference? (Your "You'd write this as" version is the baseline I'm comparing it to; you can see the assembly differences between them above.)

I think iteration is somewhat of a red herring. The question applies equally to functions that take StringPiece by value.

I did some analysis (looking at assembly + logicking it out) and ended up with this doc:
https://docs.google.com/document/d/1tLO-Y85l-K-fFLi96xddqu2dXW8Hn8Sj-kgllPG5YBM/edit
(I think writing C-equivalent code of the ASM is much easier to read than pasting in raw ASM generated by the compiler.)

The conclusion I came to is that if you write a function that takes a StringPiece, it will be inefficient to call a StringPiece method. If you write a function that takes a const StringPiece&, it will be inefficient for somebody to call your function with a std::string. Therefore, we should make StringPiece methods take StringPiece by value, and uphold the guideline of taking StringPiece by value.

Iteration is less critical, since if the functions/methods take StringPiece by value, then it doesn't matter much whether you iterate by value or by reference. But there are minor optimizations (potentially fewer loads) if you iterate by value, so we probably should do that (if we do the above). It's also easier to have a blanket rule "treat StringPieces as primitives, like int" (which means iterate by value, not by reference), rather than having a special case for function arguments. Either way, the documentation should clarify how you should iterate over a vector<StringPiece>.

#2:
> My personal intuition is that const auto& would be better when iterating. The rationale is that
> if we're iterating through a list, the StringPiece is already somewhere in memory, so we have to
> incur the cost of the dereference to get the actual StringPiece anyway. So it's a matter of
> dereferencing upfront to get the by value copy, or dereferencing later when we use the reference
> to the StringPiece.

It's situational; if it's used only once, and passed by value, then it comes out exactly the same. If it's passed by reference, it's very bad to iterate by value because it has to be copied to the stack (!). But if it's being passed by value, and used multiple times within the loop body, it's better to iterate by value because then you only have to do a memory load once. (I'm surprised that LLVM doesn't optimize repeated loads into a single load, but it doesn't.)

> I am curious to see if changing the internal helpers to pass args by value will change anything
> though...

I did that too. Here is the assembly code:
https://gist.github.com/mgiuca/5642d9146b11b80b6a3d5f3324a6666b

In my above doc, the assemblies I linked in this bug report are equivalent to IterValToRef and IterRefToRef, respectively; this one is equivalent to IterValToVal (or IterRefToVal, same code).

The summary is it's a bit less efficient having find() take by-value (than doing by-ref iteration and having find() take by-ref). But there are other good reasons to have find() take by-value.

Since there is an owner, this bug is not untriaged. Updating the status.