CODEWITHSUNDEEP
c++stringperformance
Stan
Stan

Reputation: 38255

C++ String memory reuse optimization

I was wondering if string memory for tmp can be reuse in below code. Is its memory reallocated in every iteration? Is there any better way to deal with these kind of case?

string s, line;
map<string, string> mymap;
while(getline(file, line) {
  if(a) s = "_a";
  else if(b) s = "_b";
  string tmp = line + s;
  mymap.insert(tmp, s);
}

Upvotes: 1

Views: 1833

Answers (3)

Steve Jessop
Steve Jessop

Reputation: 279215

tmp is created and destroyed each time around the loop, and takes a copy of the string data in line. So you can get a cheap probable-improvement-and-can-hardly-be-any-worse like this:

if(a) s = "_a";
else if(b) s = "_b";
line += s;
mymap.insert(line, s);

I'd also give s the type const char*: there's not much point assigning a string once per loop that only ever contains a copy of a literal. But it does get converted to string by the call to insert, so there's not much in it either way.

A probable-improvement-and-can-hardly-be-any-worse isn't premature optimization, provided you don't damage your code's simplicity/readability/maintainability/design to achieve it. The larger the scope of line and s, the more risks there are in playing tricks with them (mutating the value and altering the type, respectively), since you could somehow mislead a reader/maintainer. Which is one of the reasons short functions are good.

In C++11 you could write mymap.insert(std::move(line), s); for another easy probable-improvement.

All that said: you might find that no matter how much unnecessary copying and allocation you do, the time taken by that is dwarfed by the time for the I/O in getline. In this case there are two very similar ways of writing the code and one of them "should" be more efficient. So you might as well use it, but don't over-value it by thinking it will necessarily make a difference.

Upvotes: 4

iikkoo
iikkoo

Reputation: 2856

Every iteration of the while loop will create and destroy your string tmp object. So, the first step is to move tmp outside the while loop, as already suggested. That way you don't have to construct a new string object each iteration. But, you still have the tmp = line + s assignment which causes memory reallocation each iteration. Using the =operator creates a copy of the arguments, and assigns the copy to the tmp string object. So, the second step is to add the suggested mymap.insert(line+s, s); that removes the need for the tmp string object.

I think one could continue this improvement some by not assigning "_a" or "_b" to string s every iteration. This could be done once, outside the while loop, and then depending on the contents of a and b different string objects could be added to your map. Something like this (n.b. this is untested):

string a = "_a";
string b = "_b";
string line;
map<string, string> mymap;
while(getline(file, line) {
  if(a) mymap.insert(line+a, a);
  else if(b) mymap.insert(line+b, b);
}

One could argue if this is good, in my opinion juanchopanza's answer is enough since it maintains readability of the code. But I think that the code above has fewer copies made.

Upvotes: 3

spin_eight
spin_eight

Reputation: 4025

Yes it is, use STD::MOVE semantics which is introduced in C++ 11 standart.

update: example

#include <iostream>
#include <utility>
#include <vector>
#include <string>
int main()
{
    std::string str = "Hello";
    std::vector<std::string> v;

    // uses the push_back(const T&) overload, which means 
    // we'll incur the cost of copying str
    v.push_back(str);
    std::cout << "After copy, str is \"" << str << "\"\n";

    // uses the rvalue reference push_back(T&&) overload, 
    // which means no strings will copied; instead, the contents
    // of str will be moved into the vector.  This is less
    // expensive, but also means str might now be empty.
    v.push_back(std::move(str));
    std::cout << "After move, str is \"" << str << "\"\n";

    std::cout << "The contents of the vector are \"" << v[0]
                                         << "\", \"" << v[1] << "\"\n";
}

Upvotes: 1

Related Questions