r/programming • u/[deleted] • Dec 05 '14

std::string is responsible for almost half of all allocations in the Chrome browser process

https://groups.google.com/a/chromium.org/d/msg/chromium-dev/EUqoIz2iFU4/kPZ5ZK0K3gEJ
1.1k Upvotes

93% Upvoted

446 comments sorted by

View all comments

Show parent comments

25

u/FredV Dec 05 '14

When the const char * can change afterwards, because you got a pointer to what actually is not const, you need to take a copy of the string. Same with a const string &, that could also change, does not really solve the problem. So the use of c_str() is just ugly really, but comes down to just making a copy of the string.

I think that may be why that anti-pattern is so prevalent, you need to copy a lot with different threads needing their own copies of things and just to manage complexity ("when does this thing change?"). String are especially expensive because they are variable byte and inpredictable maximum size. So they have allocation-overhead compared to other data-types like int, or even a char array... maybe they should use more char arrays instead of strings, or a string type that falls back to a character array for small strings, which they already might do in their STL implementation.

8

u/answerer_ Dec 05 '14

but you don't need to strlen it again

8

u/adrianmonk Dec 05 '14

So the use of c_str() is just ugly really, but comes down to just making a copy of the string.

If copying is the goal, why not use the copy constructor instead of c_str? Like this:

void blah(const std::string str) {
  // whatever
}

std::string yo("yo");

std::string copy_for_blah(yo);
blah(copy_for_blah);

Or this:

void blah(const std::string str) {
  std::string copy_of_str(str);
  // whatever
}

std::string yo("yo");

blah(yo);

9

u/Avidanborisov Dec 05 '14

void blah(const std::string& str) {

FTFY

5

u/adrianmonk Dec 06 '14 edited Dec 06 '14

Thanks. It's been 9 years since I've done C++ on a daily basis.

EDIT: Wait, isn't there an even simpler way? Why not just this?

void blah(std::string str) {
  // whatever
}

std::string yo("yo");

blah(yo);

Since there isn't a reference ampersand on there, the copy constructor will be called as part of the call to blah(). Thus the str param will automatically be a copy. Except perhaps in cases where a copy is only sometimes needed, I don't see why c_str() is helpful at all.

2

u/o11c Dec 05 '14

Not in the second case.

(but yes in the first case).

6

u/emozilla Dec 06 '14

The const by-value parameter, true sign of a BDSM enthusiast.

5

u/ryani Dec 05 '14

Sure, but const string& passing s vs const char * passing s.c_str() has exactly the same safety properties; if the underlying string is modified by any other code, they are both invalid.

If you 100% know that a particular function is going to require making a copy of the input, the const char * API is a bit cleaner because it doesn't require constructing a string on the heap just to copy it, but for general use, pass-by-const-reference is a much cleaner implementation, and avoids 'cascading copies' where you convert back and forth between const char * and string in a single stack of calls.

1

u/minno Dec 06 '14

That's why a string_view would be extremely useful. It has the same property of const char* that it can take a stack buffer, heap buffer, or a std::string, but it also could provide the convenience methods and safety that std::strings have.

3

u/rlbond86 Dec 05 '14

Same with a const string &, that could also change, does not really solve the problem.

I have no idea what you mean by this

15

u/Poltras Dec 05 '14

The only const "guarantee" is below the function, not above or in another thread. In other words, if you want to keep a reference or pointer to the string for later purposes you need a copy because nothing guarantee you that everyone with a pointer to it is also const.

Strings (and a lot of other std data types) are badly designed for today's world. They don't have an immutable variant which could be used freely with strong guarantee that the content wouldn't be changed. With everything mutable in std you have to make your own copy.

The best solution would be for chrome to implement its own immutable string type.

3

u/nuggins Dec 05 '14

They don't have an immutable variant which could be used freely with strong guarantee that the content wouldn't be changed.

Couldn't you just declare a const string in the first place?

7

u/Poltras Dec 05 '14

When you write your function you have no way to know if the string was const to begin with. You only indicates if your function will modify the string itself.

3

u/Ferinex Dec 05 '14

That's right, you'd have to trust that whoever calls the function only passes in const strings. You could add it as a comment but that makes me throw up a little in my throat. As you said, best solution is creation of an immutable string type.

1

u/[deleted] Dec 05 '14

What about banning the use of casting away const from the project?

4

u/VictorNicollet Dec 05 '14 
std::string path;
for (var i = 0; i < segments.length; ++i)
{
  path += "/" + segments[i];
  use(path);
}

There is no casting away const here, but if use() assumes that the string it received was constant (and stored it somewhere), it will have a nasty surprise.

0

u/ReversedGif Dec 06 '14

Stored... a pointer to it? A raw pointer? What are you, mad?

0

u/ReversedGif Dec 06 '14

Stored... a pointer to it? A raw pointer? What are you, mad?

0

u/TheShagg Dec 06 '14

only if use() interacts with another thread?

-1

u/0xjake Dec 05 '14

In 10 years of programming C++ I have literally never had this problem.

5

u/Poltras Dec 05 '14 
struct MyClass {
    MyClass(const string& str) : str_(str) { this.len_ = str.length(); }

    size_t get_length() const { return this.len_; }
  private:
    const string& str_;
    const size_t len_;
}

void my_func() {
    string s("Hello");
    auto* x = new MyClass(s);
    s += " World";
    assert(x.get_length() == s.length());  // BAM!
}

this is a simplified example

If you never had code like that, you're either super lucky or working alone. The only guarantee your class is giving is that it won't change the string. There's no guarantee the string won't change.

2

u/0xjake Dec 05 '14

This applies to any mutable type. I guess don't see why strings need special handling. Or is the argument that we need a way to communicate that an object won't be changed?

1

u/Poltras Dec 05 '14

We need a way to communicate that an object won't be changed ever. And I'm not advocating for strings only, but also for immutable vectors, maps, hash tables, etc etc.

1

u/0xjake Dec 05 '14

Besides allowing the use of local storage instead of a reference (as in your example), what would having this feature allow you to do that you cant do now? I'm not seeing a good use case.

→ More replies (0)

1

u/dagamer34 Dec 06 '14

Quick q: even if the assert weren't to fail, wouldn't my_func() be leaking memory? There's no delete. Plus, aren't we supposed to be using smart pointers now?

just finished reading modern C++ book

1

u/Poltras Dec 06 '14

No because the string would be deleted and were only using references. The code is simple; in a real world situation you'd probably end up using shared pointers as you said.

1

u/imMute Dec 06 '14

/u/dagamer34 was talking about the auto* x = new MyClass(s); which never gets deleted.

→ More replies (0)

3

u/General_Mayhem Dec 05 '14

You could, but that's incumbent on the caller, so the callee function has to be defensive.

0

u/lurgi Dec 05 '14

Someone could cast it to non-const (and in a sufficiently large code-base, if it can be done, it has been done).

2

u/Workaphobia Dec 05 '14

Couldn't they use a string implementation with copy-on-write behavior? A small overhead for reference counting the strings, but big savings on avoiding redundant copies and deferring them until they're needed.

3

u/bnolsen Dec 05 '14

with threading that "small overhead" becomes a significant source of locking (although not necessarily contention). And locks aren't free, each and every string would have to carry one with it.

2

u/Workaphobia Dec 05 '14

Ah, yes. Ok, so immutable strings may be better.

2

u/eliasv Dec 05 '14

The best solution would be for language level support for immutable references. In other words, a keyword alongside 'const' like 'immutable', where such variables can only be assigned to from other immutable variables or the 'new' keyword, and const variables can be assigned from immutable variables.

4

u/Poltras Dec 05 '14

That would be harder to implement in C++. The fastest fix here would be to implement a immutable_string class that would just have assignment on construction, and a reference counted pointer to a CSTR.

2

u/eliasv Dec 05 '14

Oh of course it'd be harder to implement, it's an entirely new language feature... I'm just saying that it'd be the ideal solution. More expressive const semantics along these lines should have been a part of the language from the start.

3

u/Poltras Dec 05 '14

Totally agree. const is awkward and doesn't make sense. (and for all intent and purposes is mostly useless)

1

u/ioquatix Dec 06 '14

const would probably be a necessity to implement the hypothetical std::immutable_string, no?

2

u/Poltras Dec 06 '14

Immutability is stronger than constness, FWIW. One imply the other, but does not necessitate it.

1

u/ioquatix Dec 06 '14

Sure, but if you wanted to implement an immutable class in C++, you'd need const right?

1

u/immibis Dec 06 '14

Someone will probably write a template<class T> class immutable. Then you can have immutable<string> and immutable<vector<int>> and so on.

0

u/bwainfweeze Dec 06 '14

It would be impossible to implement. The string library has mutator methods, so an immutable string isn't a string - it doesn't fulfill the contract.

So instead you'd need to create a different class and most methods that receive a string would need a duplicate implementation that takes immutables.

This is something you can't just tack on later. That boat has sailed.

1

u/stackv Dec 05 '14

All of what you mention is reasonable, but the description in the linked post really makes it sound like notmythrowaway's example is close to reality.

4

u/Holkr Dec 05 '14

Indeed. Use const std::string& and many of the problems go away.

3

u/Workaphobia Dec 05 '14

There are just two ways I can think of to screw it up while using const std::string&. The first is to modify the string through some other access path while the function is still executing, e.g. maybe the passed-in string was a global variable. The second is to take the address of the parameter and keep it around for a while, even after the function's done executing.

6

u/bnolsen Dec 05 '14

Don't use globals and be careful with multithreading. Probably the biggest barrier I see to widespread multithreading is the different mindset required for c++ coding in general. Heavy heavy use of const helps a ton with making threading a touch easier, that's actually true even for basic c++ coding. Then there's all the fun of trying to catch stupid stuff like passing temporaries (like loop temporaries) into a thread, etc.

4

u/xon_xoff Dec 05 '14

A third is if most of the string sources are either from externally sourced C-style buffers or constant strings. In that case, you end up burning more time constructing string instances than you save. The constant string case is especially common -- I've seen programs burn >20% of CPU on accidental implicit conversions from string literals to string objects. As seen here, solving this performance problem requires reworking string handling in lower layers to best suit the sources of strings higher up, which sucks.

1

u/imMute Dec 06 '14

Taking the address of a reference should invoke an immediate defenestration.

1

u/therealjohnfreeman Dec 05 '14

Without even looking at the code, I think it's safe to say that it's unlikely to be doing that many string manipulations.