0

Let's say I have this code:

void MyFunc(const std::string& param) {
  std::string maybe_altered_param;
  if (ConditionIsMet()) {
    maybe_altered_param = AlterParam(param);
  } else {
    maybe_altered_param = param; // <-- unnecessary copy 
  }

  // do stuff with maybe_altered_param
  // maybe_altered_param does not need to be modified further.
}

The AlterParam function returns a copy of the string, so when ConditionIsMet returns true, a copy is made in order to populate maybe_altered_param. However, a copy is made also when ConditionIsMet() returns false, which is suboptimal. In the second case I just want to have another name for the same object, without copies or anything of the sort.

What is the simplest way of removing the unnecessary copy in a case like this?

Ant
  • 5,151
  • 2
  • 26
  • 43
  • 1
    `std::string &&maybe_altered_param = ConditionIsMet() ? AlterParam(param) : param;` or (better in your case?) `std::string const &maybe_altered_param = ConditionIsMet() ? AlterParam(param) : param;` – EOF Dec 27 '21 at 12:05
  • 1
    In both cases, is it never modified after being set? – ChrisMM Dec 27 '21 at 12:05
  • 1
    How much does this extra copy slow things down on your modern PC with a multighz, multicore CPU? What problem does this copy create? – Sam Varshavchik Dec 27 '21 at 12:07
  • @ChrisMM Yes, thats's correct! – Ant Dec 27 '21 at 12:09
  • 2
    @EOF Would that work with no extra copies? Reading this question (https://stackoverflow.com/questions/54601265/how-to-efficiently-bind-either-an-lvalue-or-rvalue-to-the-same-reference) it looks like an extra copy would be made regardless – Ant Dec 27 '21 at 12:11
  • 4
    @EOF: ternary operator would do the copy, as `std::common_type` is `std::string`. – Jarod42 Dec 27 '21 at 12:11
  • 1
    @SamVarshavchik This function is called inside a very tight loop a very high number of times. I also expect that in practically all cases, ConditionIsMet() will return false, so making sure no extra copies occur in that branch is worth it – Ant Dec 27 '21 at 12:12
  • When a parameter may be altered I usually pass it by value, this offers more chances to be optimized rather than copying a const value in a local variable – MatG Dec 27 '21 at 12:29
  • Perhaps use string_view to make sure there is no copy? In addition, if performance is important, make sure that std::string maybe_altered_param does not allocate from the heap (small string optimization). You can allocate memory on the stack instead. – Sebastian Dec 27 '21 at 12:32
  • @MatG I do the same when the param will for sure be copied. But taking it by value (hence paying the cost a copy) when 99.9% of the times the copy is not needed just means wasting a lot of resources. – Ant Jan 04 '22 at 08:27
  • @Sebastian string_view is not going to be different than a const reference in this case, as much as copy / move semantics are concerned, right? Also, i don't control how big the string in input is, how would small string optimization be helpful here? – Ant Jan 04 '22 at 08:29
  • @Ant I was referring to the local variable maybe_altered_param; only the parameter is a reference. if the optimizer does not do magic, there is the unnecessary copy, you stated. string_view is light-weight as value-type and can be reassigned in the maybe-case. – Sebastian Jan 04 '22 at 08:39
  • No idea, how large the input strings can be or typically are in your application. And also no idea, how critical performance of MyFunc is. Short strings up to 15-22 bytes are allocated on the stack, above that there is a (a bit more costly) heap allocation: https://stackoverflow.com/questions/27631065/why-does-libcs-implementation-of-stdstring-take-up-3x-memory-as-libstdc/28003328#28003328 If the strings are (for example) typically larger than 15 bytes, but always smaller than 200 bytes, you can just use a local std::array; but best to avoid copies in the beginning. – Sebastian Jan 04 '22 at 08:40
  • @Ant In that case I'd give up on having a local copy of the parameter and try to move the altering condition before the function call. And if performance is paramount I'd seek other mechanisms to convey the informations in alternative to the costly operation of changing a string. – MatG Jan 04 '22 at 10:03

2 Answers2

6

I think your question about reference binding is misguided. It's looking for a language solution to what is essentially a code organization problem.

You want to "do stuff with maybe_altered_param", stuff you don't want to repeat for every branch (because DRY code is best code). So why not refactor that bit out?

static void MyFuncImpl(const std::string& maybe_altered_param) {
  // do stuff with maybe_altered_param
}

void MyFunc(const std::string& param) {
  if (ConditionIsMet()) {
    MyFuncImpl(AlterParam(param));
  } else {
    MyFuncImpl(param);
  }
}

Easy peasy. The extra copy is made only when the condition is met, and the stuff we do with maybe_altered_param is only spelled out once, parameterized by that parameter.

May not be the answer you are after, but I think it's worth considering.

StoryTeller - Unslander Monica
  • 165,132
  • 21
  • 377
  • 458
  • That's definitely worth considering, but the code is not quite as reported in my question. There is some setup before conditionIsMet() is called, and I would need to pass a few variables into MyFuncImpl. i think this might easily lead to code which is harder to read. Is there a way perhaps to do the inverse, e.g. const std::string& maybe_altered = MaybeAlterParam() and rely on lifetime extension to make it work? – Ant Dec 27 '21 at 12:19
  • I am not quite 100% sure if lifetime extension would work in this case, however :) – Ant Dec 27 '21 at 12:20
  • @Ant - I can only address the code I see. And lifetime extension won't save you here, because extension only applies to a new object. You can't plug `param` there instead (this leads to your original question, where a copy must happen). If your setup is more complex, then you can still do other refactorings. A small helper class that holds the shared information (so you do not need to pass it), and which has a member function taking a string parameter. Could even be a lambda (where you reference capture the shared data). – StoryTeller - Unslander Monica Dec 27 '21 at 12:23
  • What I meant was, would something like this work? const std::string& MaybeAlterParam(const std::string& param) { if (ConditionIsMet()) {return AlterParam(param);} return param; } And then use it as void MyFunc(const std::string& param) { const std::string& maybe_altered_param = MaybeAlterParam(param); } In case where ConditionIsMet is false, no copy happens as the reference is returned. If it's true, will the lifetime of the string created inside MaybeAlterParam be extended until maybe_altered_param goes out of scope? – Ant Dec 27 '21 at 14:57
  • @Ant - No, you are returning a reference to a temporary here. – StoryTeller - Unslander Monica Dec 27 '21 at 14:58
  • Wouldn't the lifetime of the temporary be extended in that case, however? e.g. https://abseil.io/tips/107 I am sure I am missing something, but it seems to me that it would (at least if I declare the return value of MaybeAlterParam as simply std::string, and rely on other compilers optimization to make sure no copies happen on the 'normal path'). Alternatively, would this be a solution? std::string alternative; if (ConditionIsMet()) {alternative = AlterParams(param);} const std::string& maybe_alt_parram = ConditionIsMet() ? alternative : param; IIUC no copy would happen here, right? – Ant Dec 27 '21 at 15:14
  • Binding a reference to a temporary extends it to the lifetime of the reference. But returning said reference (even if the temporary is materialzed in the return statement) does not prolong the temporary, – StoryTeller - Unslander Monica Dec 27 '21 at 15:19
2

With extra variables, you might do:

void MyFunc(const std::string& param) {
    std::string maybe_altered_param;
    const bool condition_met = ConditionIsMet();
    if (condition_met) {
        maybe_altered_param = AlterParam(param);
    }
    const std::string& ref =
        condition_met ? maybe_altered_param : param; // both are lvalues, so no copy.
    // do stuff with ref
}
Jarod42
  • 203,559
  • 14
  • 181
  • 302
  • +1 yes, that is what I was thinking as well! Is there an easy way to check that this indeed involves no copy when condition_met is false? – Ant Dec 27 '21 at 23:52
  • Change `std::string` by a type which tracks/logs constructor calls [Demo](https://coliru.stacked-crooked.com/a/a146f978e386fbf6). – Jarod42 Dec 28 '21 at 00:13
  • So nice, thank you! :) – Ant Dec 28 '21 at 06:02