string operator= reallocates unnecessarily

[vitek]

Ha. You'd think so, but then you'd be wrong. There is no good way to efficiently assign a string without causing it to be reallocated. Have a look at the defintion of core::string<>::operator=(). There is no check to see if existing buffer needs to be reallocated or not. Sadly, the only place that the capacity is actually checked is in append(), but since there isn't even a (hacky) way to set the number of used elements back to zero, you can't even use that.

Regardless, the operator+=() call will result in the creation of an additional temporary that is allocated on the heap. If someone cared about the overhead they'd probably use a buffer and one of the sprintf() C library functions.

Travis

[JP]

Could the check not be added in?

So i suppose the best thing to use is to allocate a char array of a large enough size for any use you want to use it for outside the loop and then sprintf into it each frame.

But as i said before this is going a bit too far for 3DMod's requirements so he can just use your code heh.

[vitek]

It absolutely should be added. Actually, core::string<> should be rewritten so it doesn't suck so bad. Unfortunately I'm not volunteering to make the changes, I just like complaining aloud. :)

Travis

[rogerborg]

It absolutely should be added. Actually, core::string<> should be rewritten so it doesn't suck so bad. Unfortunately I'm not volunteering to make the changes, I just like complaining aloud.

You say that now, but it's going to gnaw away at you, like a spiny alien maggot in your mind. Late at night, you'll be thinking "Why has nobody fixed the string class yet? It's an easy fix. I bet I - I mean, someone - could even knock up a unit/regression test to validate it really quickly."

Gnaw, gnaw.



Gnaw, gnaw, gnaw.

[hybrid]

I think Thomas (burningwater) has some things fixed regarding this issue. We just need to wait for his commit (to trunk )

[rogerborg]

I have a candidate fix for operator=, with a unit/regression test. It's not a new feature per se, but neither is it properly a bug, and it could result in merry hell if I get it wrong.

Want to risk it in 1.5, or shall I hold off and put it (just) on the trunk once we're done merging the 1.5 changes down?

[UPDATE]
We've agreed to hold it back until 1.5.1.

[vitek]

I can fix the problems with core::string<>, but I'm not promising it will be done anytime soon... :)

[rogerborg]

I've dumped my current patch in the tracker, so knock yourself out. Laudenum works a treat.