inet_address: Remove to_sstring() in favor of fmt::to_string

[xemul]

The existing inet_address::to_string() calls fmt::format("{}", *this) anyway. However, the to_string() method is declared in .cc file, while form formatter is in the header and is equipeed with constexprs so that converting an address to string is done as much as possible compile-time.

Also, though minor, fmt::to_string(foo) is believed to be even faster than fmt::format("{}", foo).

[nyh]

Your patch is lacking motivation: In classic OO programming, a method is better than a global function. Among other things, when you have a method on a method ip.to_sstring() instead of a free function fmt::to_string(ip) you have an opportunity to document what this to_sstring() actually does, what format it promises to uses, and so on. So what is the motivation to go the other way - replace a method by a free function?

Another result of your change is that we convert the address into an std::string instead of a seastar::string. Is this good? bad? It should at least be discussed and I would have liked to see some motivation for the change.

An example of the std::string vs. seastar::sstring issue is the following code (after your patch):

std::map<sstring, sstring> nodes_status;
g.for_each_endpoint_state([&] (const gms::inet_address& node, const gms::endpoint_state&) {
       nodes_status.emplace(fmt::to_string(node), g.is_alive(node) ? "UP" : "DOWN");

Note how you build a map of seastar::sstring, yet you put into it a std::string returned by fmt::to_string(). This requires unnecessary conversions between the two. In this case you should probably make the map have a std::string key instead of sstring. It's not a bad idea (in Alternator I also moved away from sstring to std::string as much as possible), but I think it needs to be done more carefully.

[xemul]

The existing inet_address::to_string() calls fmt::format("{}", *this) anyway. However, the to_string() method is declared in .cc file, while formatter is in header and is equipeed with constexprs so that converting an address to string is done as much as possible compile-time. Also, though minor, fmt::to_string(foo) is believed to be even faster that fmt::format("{}", foo).

[xemul]

upd:

• added motivation to patch commit message

[xemul]

Regarding to_string() method. Having formatter is beneficial, because if you have log.debug("{}", object.to_string()) the conversion to string happens even for low enough log-level, but log.debug("{}", object) when the object has formatter doesn't convert object to string when not needed

[tchaikov]

lgtm

[scylladb-promoter]

🟢 CI State: SUCCESS

✅ - Build
✅ - Container Test
✅ - dtest
✅ - dtest with topology changes
✅ - Unit Tests

Build Details:

• Duration: 3 hr 48 min
• Builder: i-0a83f309df996450d (m5d.12xlarge)