3

I'm working in a codebase with a mixture of CString, const char* and std::string (non-unicode), where all new code uses std::string exclusively. I've now had to do the following:

{
    CString tempstring;
    load_cstring_legacy_method(tempstring);
    stdstring = tempstring;
}

and worry about performance. The strings are DNA sequences so we can easily have 100+ of them with each of them ~3M characters. Note that adjusting load_cstring_legacy_method is not an option. I did a quick test:

// 3M
const int stringsize = 3000000;
const int repeat     = 1000;

std::chrono::steady_clock::time_point startTime = std::chrono::steady_clock::now();
for ( int i  = 0; i < repeat; ++i ){
    CString cstring('A', stringsize);
    std::string stdstring(cstring); // Comment out
    cstring.Empty();
}
std::cout << std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::steady_clock::now() - startTime).count() << " ms" << std::endl;

and commenting out the std::string gives 850 ms, with the assignment its 3600 ms. The magnitude of the difference is suprising so I guess the benchmark might not be doing what I expect. Assuming there is a penalty, is there a way I can avoid it?

Blacktempel
  • 3,935
  • 3
  • 29
  • 53
Adversus
  • 2,166
  • 20
  • 23

3 Answers3

1

So your question is to make the std::string construction faster?

On my machine, comparing this

std::string stdstring(cstring); // 4741 ms

I get better performance this way:

std::string stdstring(cstring, stringsize); // 3419 ms

or if the std::string already exists like the first part of your question suggests:

stdstring.assign(cstring, stringsize); // 3408 ms
acraig5075
  • 10,588
  • 3
  • 31
  • 50
  • Any speed gain is welcome. I don't know anything about the `std::string` nor `CString` internals, I wondered if there is a way to avoid the penalty alltogether. Note that the penalty is so big that I'm considering adding a `load_cstring_legacy_method` overload that takes a std::argument. – Adversus Mar 10 '15 at 14:54
  • @Adversus Maybe tag your question as c++ to get exposure to more members. Only `chrono` in your question is c++11. – acraig5075 Mar 10 '15 at 15:00
  • ah right, ok. Also note that I checked and allocating a "fresh" `std::string` or `CString` takes the same time. Was wondering if `std::string` was somehow just slow, but the 2800 ms is must be in the allocation (~800ms) and data copying (~2000 ms). – Adversus Mar 10 '15 at 15:07
0

Use a more efficient memory allocator. Something like a memory arena/region would substantially help with allocation costs.

If you're really, really desperate, you could theoretically combine ReleaseBuffer with some hideous allocator hacks to avoid the copy altogether. This would involve a lot of pain, though.

In addition, if you have a serious problem, you could consider changing your string implementation. The std::string that ships with Visual Studio employs SSO, or Small String Optimization. This does exactly what it sounds like- it optimizes very small strings, which are quite common all around but not necessarily good for this use case. Another implementation like COW could be more appropriate (be super careful if doing so in a multi-threaded environment).

Finally, if you're using an old version of VS, you should also consider upgrading. Move semantics are a huge instawin as far as performance goes.

Puppy
  • 144,682
  • 38
  • 256
  • 465
  • Thanks for the pointers. It's VS 2012. The codebase is too big to consider switching string implementations unfortunately. I hoped someone would already be using a hack like (ab)using `ReleaseBuffer` or the likes that would be a quick win. – Adversus Mar 10 '15 at 15:47
  • @Adversus: I actually looked on MSDN and ReleaseBuffer does not, in fact, release the buffer at all, so there's nothing available for you here but to optimize things on the std::string side with a custom allocator. – Puppy Mar 10 '15 at 15:50
  • Yeah, you're right. Maybe it's possible using a custom `IAtlStringMgr` that uses (and keeps track of) a std::string under the hood which can swapped out. The only operation on the `CString` in `load_cstring_legacy_method` is `cstring = `. – Adversus Mar 10 '15 at 16:55
  • Hit a dead end. Using a custom `IAtlStringMgr` I was able to put the cstring data into a `char*` of my choosing. `CStringData` requires this memory to be located next to it: `CStringData::data{ return this+1; }`. This works with `char*` but not with `std::string` as I have no control over the memory allocation (no way to specify a custom allocator apparently). Nor do I know of a way to "move" my `char*` into a `std::string`. – Adversus Mar 11 '15 at 09:04
  • @Adversus: You can use a custom allocator with `std::string` that can move from a `char*` into a `std::string`. `std::basic_string` is the underlying template which is fully allocator-aware. `std::string` is just a convenience typedef. – Puppy Mar 12 '15 at 11:54
  • Thanks for the tip, I'll have to look into that. I don't immediately see how my custom `std::basic_string` would be compatible with the `std::string` that we are using throughout the code (without triggering a copy). – Adversus Mar 13 '15 at 12:45
0

CString is probably the Unicode version, which explains the slowness. The generic conversion routine cannot know assume that the characters used are limited to "ACGT".

You can, however, and shamelessly take advantage of that.

{
    CString tempstring;
    load_cstring_legacy_method(tempstring);
    int len = tempstring.GetLength();
    stdstring.reserve(len);
    for(int i = 0; i != len; ++i)
    {
        stdstring.push_back(static_cast<char>(tempstring[i]));
    }
}

Portable? Only so far as CString is, so Windows variants.

MSalters
  • 173,980
  • 10
  • 155
  • 350
  • Tried this in the benchmark above, with and without the `static_cast`, got `29500 ms`. So no dice. No unicode in the code whatsoever btw. – Adversus Mar 10 '15 at 17:02
  • Well, the `static_cast` is just for readability, so that shouldn't matter. But this measurement probably means you're not running a Unicode build. – MSalters Mar 10 '15 at 22:52