Performance improvements

[rpopescu]

No description provided.

[Enmk]

Ok

[rpopescu]

@Enmk how does creating two strings in a loop not have any impact? In what test? In my real world use-case, it made a significant difference. It won't make one only if the strings can benefit from SSO, which is dependent on the values that they take at runtime (and having a SSO implementation of std::string).

[alexey-milovidov]

+1 for @rpopescu

[Enmk]

@Enmk how does creating two strings in a loop not have any impact? In what test? In my real world use-case, it made a significant difference. It won't make one only if the strings can benefit from SSO, which is dependent on the values that they take at runtime (and having a SSO implementation of std::string).

@rpopescu I actually tested that against 9000-columns wide table and found no significant difference. But could you please share your real-life experience (and compiler\toolset version too)?

[alexey-milovidov]

@Enmk you will always note the difference in tight loops if std::string is allocating memory. It is not a question... note that std::string also memsets the memory region.

[alexey-milovidov]

But it may not be noticed due to overall substandard performance of clickhouse-cpp driver.

[alexey-milovidov]

Or it's not a tight loop, then it's ok.

[Enmk]

@rpopescu @alexey-milovidov
My point:
This is hardly a tight loop, there are lots of things going on besides those std::string instances being constructed\destructed. Namely, creating and de-serializing a whole column. My estimation was that those std::string take less than 1% of execution time at best, and my tests unfortunately confirm this on reading a pretty wide (9000 columns) and pretty shallow (1 row) block.

BUT to make you both happy, I've moved those outside of the for loop. And please let's keep discussion civilized, and for performance that means numbers, not opinions.

But it may not be noticed due to overall substandard performance of clickhouse-cpp driver.

Agree it is not ideal and we are trying little by little to improve it, (specifically, ColumnString and ColumnFixedString got much better during 2020). I'm all in on improving performance, but unfortunately this simple trick doesn't help at all in this case.