Optimizing string creation

Question

I have the following mock up code of a class which uses an attribute to set a filename:

#include <iostream>
#include <iomanip>
#include <sstream>

class Test {
    public:
        Test() { id_ = 1; }
        /* Code which modifies ID */

        void save() {
            std::string filename ("file_");
            filename += getID();
            std::cout << "Saving into: " << filename <<'\n';
        }

    private:
         const std::string getID() {
             std::ostringstream oss;
             oss << std::setw(4) << std::setfill('0') << id_;
             return oss.str();
         }

         int id_;
};

int main () {
    Test t;
    t.save();
}

My concern is about the getID method. At first sight it seems pretty inefficient since I am creating the ostringstream and its corresponding string to return. My questions:

1) Since it returns const std::string is the compiler (GCC in my case) able to optimize it?

2) Is there any way to improve the performance of the code? Maybe move semantics or something like that?

Thank you!

Answer 1

1) Since it returns const std::string is the compiler (GCC in my case) able to optimize it?

Makes no sense to return a const object unless returning by reference

2) Is there any way to improve the performance of the code? Maybe move semantics or something like that?

Id id_ does not change, just create the value in the constructor, using an static method may help:

     static std::string format_id(int id) {
         std::ostringstream oss;
         oss << std::setw(4) << std::setfill('0') << id;
         return oss.str();
     }

And then:

Test::Test()
 : id_(1)
 , id_str_(format_id(id_))
{ }

Update:

This answer is not totally valid for the problem due to the fact that id_ does change, I will not remove it 'cause maybe someone will find it usefull for his case. Anyway, I wanted to clarify some thoughts:

• Must be static in order to be used in variable initialization
• There was a mistake in the code (now corrected), which used the member variable id_.
• It makes no sense to return a const object by value, because returning by value will just copy (ignoring optimizations) the result to a new variable, which is in the scope of the caller (and might be not const).

My advice

An option is to update the id_str_ field anytime id_ changes (you must have a setter for id_), given that you're already changin the member id_ I assume there will be no issues updating another.

This approach allows to implement getID() as a simple getter (should be const, btw) with no performance issues, and the string field is computed only once.

Answer 2

Creating an ostringstream, just once, prior to an expensive operation like opening a file, doesn't matter to your program's efficiency at all, so don't worry about it.

However, you should worry about one bad habit exhibited in your code. To your credit, you seem to have identified it already:

1) Since it returns const std::string is the compiler (GCC in my case) able to optimize it?

2) Is there any way to improve the performance of the code? Maybe move semantics or something like that?

Yes. Consider:

class Test {
    // ...
    const std::string getID();
};

int main() {
    std::string x;
    Test t;
    x = t.getID();  // HERE
}

On the line marked // HERE, which assignment operator is called? We want to call the move assignment operator, but that operator is prototyped as

string& operator=(string&&);

and the argument we're actually passing to our operator= is of type "reference to an rvalue of type const string" — i.e., const string&&. The rules of const-correctness prevent us from silently converting that const string&& to a string&&, so when the compiler is creating the set of assignment-operator functions it's possible to use here (the overload set), it must exclude the move-assignment operator that takes string&&.

Therefore, x = t.getID(); ends up calling the copy-assignment operator (since const string&& can safely be converted to const string&), and you make an extra copy that could have been avoided if only you hadn't gotten into the bad habit of const-qualifying your return types.

---

Also, of course, the getID() member function should probably be declared as const, because it doesn't need to modify the *this object.

So the proper prototype is:

class Test {
    // ...
    std::string getID() const;
};

---

The rule of thumb is: Always return by value, and never return by const value.

Answer 3

You could change getID in this way:

std::string getID() {
  thread_local std::ostringstream oss;
  oss.str("");  // replaces the input data with the given string
  oss.clear();  // resets the error flags

  oss << std::setw(4) << std::setfill('0') << id_;
  return oss.str();
}

it won't create a new ostringstream every single time.

In your case it isn't worth it (as Chris Dodd says opening a file and writing to it is likely to be 10-100x more expensive)... just to know.

Also consider that in any reasonable library implementation std::to_string will be at least as fast as stringstream.

1) Since it returns const std::string is the compiler (GCC in my case) able to optimize it?

There is a rationale for this practice, but it's essentially obsolete (e.g. Herb Sutter recommended returning const values for non-primitive types).

With C++11 it is strongly advised to return values as non-const so that you can take full advantage of rvalue references.

About this topic you can take a look at:

• Purpose of returning by const value?
• Should I return const objects?

Answer 4

One possibility would be to do something like this:

std::string getID(int id) { 
    std::string ret(4, '0') = std::to_string(id);

    return ret.substring(ret.length()-4);
}

If you're using an implementation that includes the short string optimization (e.g., VC++) chances are pretty good that this will give a substantial speed improvement (a quick test with VC++ shows it at around 4-5 times as fast).

OTOH, if you're using an implementation that does not include short string optimization, chances are pretty good it'll be substantially slower. For example, running the same test with g++, produces code that's about 4-5 times slower.

One more point: if your ID number might be more than 4 digits long, this doesn't give the same behavior--it always returns a string of exactly 4 characters rather than the minimum of 4 created by the stringstream code. If your ID numbers may exceed 9999, then this code simply won't work for you.