Change RedisConnection::Query::value_type from String to std::variant<const char*,String>

[Al2Klimov]

(Find TL;DR below)

Seriously speaking, 70ca88a pollutes the diff and should be viewed on its own. It just replaces std::vector<String> with RedisConnection::Query. As RedisConnection::Query already was a std::vector<String>, this is a no-op on its own. But it allows to just change RedisConnection::Query itself and affect all its usages at once.

Why do we have to change String to std::variant<const char*,String> at all? (efe5f22)

Especially our history messages contain lots of hardcoded C string literals "like this one". At runtime, they get translated to pointers to constant global memory, const char*. String malloc(3)s and copies these data every time. In contrast, std::variant<const char*,String> just stores the address if any.

TL;DR

• The JSON RPC message processing involves Icinga DB handlers
• The Icinga DB handlers assemble Redis messages, std::vector<T>
• The faster T is, the better for JSON RPC message processing
• The fastest possible T is const char*, if any, which is just a pointer

fixes #10276
ref/NC/820479

[Al2Klimov]

Strange, it worked on my machine...

[julianbrost]

The idea looks fine in general. Besides getting it to work, you might want to take a look at std::string_view. If you implement a conversion to it once, that should allow to avoid other case-distinctions (as the purpose of string_view is to abstract how exactly the backing string is stored).

[Al2Klimov]

Like, class TODO : public std::variant<const char*,String> { operator std::string_view(); };?

[julianbrost]

Probably something like that. A function for it might work too, but a small class could be nicer overall.

[Al2Klimov]

Fine, @yhabteab, you have won...