[container.string] why isn't memcpy used in the internals?
Hi! Recently I've been trying to improve the performance of a code that makes heavy use of std::string (lots of constructions and destructions). My first thought was to use boost::container::string as it does not use reference-counting and uses small-string optimization, so it should be a perfect replacement for std::string when it comes to performance. Right... But the problem is that switching to boost::container::string only made things worse in the general case. I wrote "in the general case" because when strings to be held are small and SSO may be applied then boost::container::string outperforms std::string considerably. However, in the general case constructing boost::container::string from a C string is significantly slower than constructing std::string. My investigation showed that std::string uses memcpy to copy the C string into its buffer while boost::container::string does not. The memcpy comes from std::char_traits::copy so maybe boost::container::string could use it too? I guess the rest of operations from std::char_traits (find, move, assign etc.) could be utilized too as std::char_traits has been carefully crafted with performance in mind. Should I create a ticket for this issue? Best regards, Adam Romanek
El 10/02/2014 9:18, Adam Romanek escribió:
Hi!
Recently I've been trying to improve the performance of a code that makes heavy use of std::string (lots of constructions and destructions). My first thought was to use boost::container::string as it does not use reference-counting and uses small-string optimization, so it should be a perfect replacement for std::string when it comes to performance.
Right... But the problem is that switching to boost::container::string only made things worse in the general case. I wrote "in the general case" because when strings to be held are small and SSO may be applied then boost::container::string outperforms std::string considerably. However, in the general case constructing boost::container::string from a C string is significantly slower than constructing std::string.
My investigation showed that std::string uses memcpy to copy the C string into its buffer while boost::container::string does not. The memcpy comes from std::char_traits::copy so maybe boost::container::string could use it too? I guess the rest of operations from std::char_traits (find, move, assign etc.) could be utilized too as std::char_traits has been carefully crafted with performance in mind.
Should I create a ticket for this issue?
Yes, please. I'm slowly trying to benchmark and improve Boost.Container performance, I haven't started optimizing basic_string so your ticket could be a good start point. In a quick inspection I see that the constructor is dispatched to basic_string::assign(It, It) and that assignment is not optimized at all. I should definitely write a assignment function that dispatches to std::char_traits::copy, as it could be used to optimize several member functions. Best, Ion
On 02/10/2014 09:54 AM, Ion Gaztañaga wrote:
El 10/02/2014 9:18, Adam Romanek escribió:
Should I create a ticket for this issue?
Yes, please. I'm slowly trying to benchmark and improve Boost.Container performance, I haven't started optimizing basic_string so your ticket could be a good start point.
For those interested see [1] for the created ticket. I also provided a patch with a fix proposal. Best regards, Adam Romanek [1] https://svn.boost.org/trac/boost/ticket/9648
El 11/02/2014 8:46, Adam Romanek escribió:
On 02/10/2014 09:54 AM, Ion Gaztañaga wrote:
El 10/02/2014 9:18, Adam Romanek escribió:
Should I create a ticket for this issue?
Yes, please. I'm slowly trying to benchmark and improve Boost.Container performance, I haven't started optimizing basic_string so your ticket could be a good start point.
For those interested see [1] for the created ticket. I also provided a patch with a fix proposal.
Best regards, Adam Romanek
Thanks for the patch. I think it's missing a final step: it should set the size of the container: this->priv_size(n);
On 02/11/2014 02:13 PM, Ion Gaztañaga wrote:
Thanks for the patch. I think it's missing a final step: it should set the size of the container:
this->priv_size(n);
Ion, you're absolutely right, I missed it somehow. I attached an updated patch to the ticket so that anyone willing to apply it would not get a broken patch. Best regards, Adam Romanek
participants (2)
-
Adam Romanek
-
Ion Gaztañaga