Your function modifies the get/put position of the buffer. I think this is a bug, and it makes benchmarks misleading. Try calling extract
twice in a row -- you'll get different results. You set the put position after calling sgetn
but you should have set the get position. This should be intuitive. After a read, you "undo" your reading. Fix the bug with something like this:
string extract() {
streambuf& buffer = *str_buffer.rdbuf();
vector<char> sequence(str_buffer.tellp());
auto g = str_buffer.tellg();
buffer.sgetn(sequence.data(), str_buffer.tellp());
str_buffer.seekg(g); // seekg to the original position
// as far as I can work out, buffer.pubseekpos is not necessary here?
return string(ALL(sequence));
}
Your function still does not compute the same thing as str
. str
returns the entire buffer. Your function returns from the input position to the output position. These are different -- try calling str_buffer.seekg(3)
before comparing the output of the two functions if you want to see for yourself. You could seekg(0)
before you call sgetn
:
auto g = str_buffer.tellg();
str_buffer.seekg(0);
buffer.sgetn(sequence.data(), str_buffer.tellp());
str_buffer.seekg(g);
I think this implementation is now correct enough to benchmark. I tried the updated version vs str
on http://quick-bench.com. I used clang-9 and libstdc++(GNU). I tried a stringstream
with 1000 characters and also 10,000 characters.
This implementation is about 130 times slower than str
with no optimizations and with -O1/Clang, and it is and 3.5 times slower with -O2/Clang.
How can you speed up your function? Well the easiest way is to use str
! But let's make some small improvements. You copy the buffer into a vector<char>
and then copy the vector into a string. What if we only have one string?
string extractOneString() {
std::string sequence;
sequence.resize(str_buffer.tellp());
auto g = str_buffer.tellg();
str_buffer.seekg(0);
str_buffer.rdbuf()->sgetn(sequence.data(), str_buffer.tellp());
str_buffer.seekg(g);
return sequence;
}
Now the unoptimized version is 3.5 times slower -- just like the optimized version before. As it turns out, the Clang's -O2 was already making this change for you.
Still, 3 times slower is not good.
This is maybe not obvious, but if you look at the assembly (perhaps using http://godbolt.org), you'll notice that resize
calls memset to set the new buffer to zero. That's not necessary since you're about to overwrite the buffer anyway. There's no easy way to get around this (unless you count using str
). I think this contributes to the 3x slowdown (not sure and I don't think it's worth confirming).
A few notes about your code:
Don't use using namespace std
. People have written about this in lots of places so I won't here.
The ALL
macro is ugly and unnecessary. If you cannot stand to write iterators, look into the new ranges library.
It's worth learning sooner rather than later not to spend time optimizing the wrong thing. You wrote this code and then (I) measured it. Next time, measure your code first and then decide whether it's worth changing. This has the added advantage that you're already set up to measure the code post change.
I think it's also worth focusing on the basic semantics of your program before worrying too much about measured performance. An experienced C++ dev would see right away that your extract
function has a unnecessary copies and would most likely fix that before profiling. Learning to avoid copies and other idioms will help you avoid having to measure every little thing.
return string(ALL(sequence));
doesn't take a copy? \$\endgroup\$