search

scylladb / seastar

High performance server-side application framework
http://seastar.io
Apache License 2.0
8.29k stars 1.54k forks source link

basic_sstring should have`push_back()` #2104

Open tchaikov opened 7 months ago

tchaikov commented 7 months ago

we need to implement constexpr void push_back(char_type ch); for basic_sstring<..>.

tchaikov commented 7 months ago

i found this when trying to use something like

sstring s;
fmt::format_to(std::back_inserter(s), "{}", 1);
avikivity commented 7 months ago

This is a bad use case, since it will have quadratic behavior.

Maybe it's a bad idea to implement push_back as it encourages bad performance.

nyh commented 7 months ago

This is a bad use case, since it will have quadratic behavior.

It shouldn't - according to https://en.cppreference.com/w/cpp/string/basic_string/push_back, the push_back() function has "Amortized constant" complexity, which means that pushing N characters has O(N) complexity, not O(N^2).

Maybe it's a bad idea to implement push_back as it encourages bad performance.

I realize it's harder to do the longer we wait, but I really think we should think about drop sstring and use std::string (i.e., issue https://github.com/scylladb/seastar/issues/634). I don't say we should do it today, but it's something we should start considering (and should have considered many years ago). C++23 added even more features to std::string which seastar::sstring doesn't implement, and eventually we'll need :-(