A mini-break in C++ Optimization (Pt 1)

I have had one of those evenings, in reviewing some code for a friend I came across a repeated pattern where he would iterate over collections, strings and other linear items in a loop and mutate them.  We're of course talking C++ here, and he had this whole myriad of loops all over sanitizing, processing, event summing values.

I asked whether he had access to the STL he immediately got a bit uppity, he and I have never seen eye to eye about using the STL.

I advocate it be used, even just standing something up which you go back and make more performant or more memory efficient later, just stand up your project, don't get bogged down in details of the best string, or the best vector.

He however stands by his having "long tested examples I use all the time, just ignore them"...

No.

Simply put his examples, used in a smattering of programs, peer reviewed very infrequently by a miniscule handful of people is not good enough, use the STL, it is seen by literally millions of engineers, it is very portable and well adopted, use it to your advantage.

We sparred a little about this, but then I simply asked whether he had any parallelism in his algorithms?

Nope, of course STL can just use them.... You can provide the execution policy.

Lets take one of his examples, a simple function to give him the time & date as a character array.

char* GetDateTimeStringHis()
{
time_t now = time(0);
char* buffer = new char[64];
memset(buffer, 0, 64);
ctime_s(buffer, 63, &now);
for (int i = 0; i < 64; ++i)
{
if (buffer[i] == '\n') buffer[i] = 0;
}
return buffer;
}

This returns a character array buffer raw pointer.... No, he interjected, it returns a string.  A raw pointer to memory is NOT a string folks, this is where my argument on that point begins and ends.  This code is very unsafe, I don't like it and I certainly insist it can easily leak memory, for whom is going to delete that buffer?

So I erase that buffer with an actual standard string:

        std::string GetDateTimeStringV1()
{
time_t now = time(0);
std::string buffer(64, 0);
ctime_s(buffer.data(), 63, &now);
for (int i = 0; i < 64; ++i)
{
if (buffer[i] == '\n') buffer[i] = 0;
}
return buffer;
}

he didn't like this, but accepted perhaps that was better, he argued about the string constructor taking a length and a character (zero) to initialize itself with, but went with it.  It's safer, simpler, easier to read code.

I'm still not happy, I don't like that loop:

        std::string GetDateTimeStringV1()
{
time_t now = time(0);
std::string buffer(64, 0);
ctime_s(buffer.data(), 63, &now);
std::transform(
buffer.begin(), buffer.end(), buffer.begin(),
[](const char character) -> char
{
if (character == '\n')
{
return 0;
}
else
{
return character;
}});
return buffer;
}

He hated this, and I have to be honest I don't like the lambda either, but this is correct, using the transform algorithm and the lambda could be expressed in a location it could be reused, or just as a free function somewhere, it doesn't have to be inline here making a mess of reading the code.

What however is it doing?  It changes any new line to a zero in the buffer string, why?  Well ctime_s puts a new line at the end, so if we reverse the iteration we may get a much better performance we can early out of the loop.  But we can even just do away with the whole transform, we know it's the last character so just set it to zero as an optimization and simplification.  My friend can get behind this now:

        std::string GetDateTimeStringV2()
{
time_t now = time(0);
std::string buffer(64, 0);
ctime_s(buffer.data(), 63, &now);
buffer[std::strlen(buffer.data())-1] = 0;
return buffer;
}

We can even further think about this code, what will strlen do?  It might count from 0 to N to find the first zero character as the value, could we therefore count backwards from the 64th position in the buffer?  Well, we know the format of the data and time we get is known:

        Wed Mar 29 23:36:01 2023

Twenty five characters with the new line, adding another for the null termination character, so we can always just set the twenty fourth to zero and reduce the buffer size at the same time to perfectly fit!

        std::string GetDateTimeStringV3()
{
time_t now = time(0);
std::string buffer(26, 0);
ctime_s(buffer.data(), 26, &now);
buffer[24] = 0;
return buffer;
}

I feel I'm getting somewhere, I just want to make sure we use the STL version of the time functions now and use brace initialization for the "now".

        std::string GetDateTimeStringV4()

{

std::time_t now{ std::time(0) };

std::string buffer(26, 0);

ctime_s(buffer.data(), 26, &now);

buffer[24] = 0;

return buffer;

}

This ended my concerns and improved the performance of the code, he was calling this function nearly every time he logged, nearly every time he sent a network message and for very many of his database transactions.  This was 10 minutes of just thinking about the problem.

Now... Back to <algorithm> and execution policy with him...