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

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. (Actually, const char* is wrapped by std::string_view to not compute its length every time.)

Author: Al2Klimov

lib/icingadb/icingadb-objects.cpp

@@ -383,7 +383,15 @@ void IcingaDB::UpdateAllConfigObjects()
 			upqObjectType.Enqueue([&]() {
 				for (auto& hMSet : source.second) {
 					for (decltype(hMSet.size()) i = 0, stop = hMSet.size() - 1u; i < stop; i += 2u) {
-						dest.emplace(std::move(hMSet[i]), std::move(hMSet[i + 1u]));
+						auto variantToString = [](RedisConnection::QueryArg v) -> String {
+							if (auto str (std::get_if<String>(&v.GetData())); str) {
+								return std::move(*str);
+							}
+
+							return std::get<std::string_view>(v.GetData());
+						};
+
+						dest.emplace(variantToString(std::move(hMSet[i])), variantToString(std::move(hMSet[i + 1u])));
 					}
 
 					hMSet.clear();
@@ -691,12 +699,12 @@ void IcingaDB::InsertObjectDependencies(const ConfigObject::Ptr& object, const S
 			auto id (HashValue(new Array({m_EnvironmentId, actionUrl})));
 
 			if (runtimeUpdate || m_DumpedGlobals.ActionUrl.IsNew(id)) {
-				actionUrls.emplace_back(std::move(id));
+				actionUrls.emplace_back(id);
 				Dictionary::Ptr data = new Dictionary({{"environment_id", m_EnvironmentId}, {"action_url", actionUrl}});
 				actionUrls.emplace_back(JsonEncode(data));
 
 				if (runtimeUpdate) {
-					AddObjectDataToRuntimeUpdates(runtimeUpdates, actionUrls.at(actionUrls.size() - 2u), m_PrefixConfigObject + "action:url", data);
+					AddObjectDataToRuntimeUpdates(runtimeUpdates, id, m_PrefixConfigObject + "action:url", data);
 				}
 			}
 		}
@@ -706,12 +714,12 @@ void IcingaDB::InsertObjectDependencies(const ConfigObject::Ptr& object, const S
 			auto id (HashValue(new Array({m_EnvironmentId, notesUrl})));
 
 			if (runtimeUpdate || m_DumpedGlobals.NotesUrl.IsNew(id)) {
-				notesUrls.emplace_back(std::move(id));
+				notesUrls.emplace_back(id);
 				Dictionary::Ptr data = new Dictionary({{"environment_id", m_EnvironmentId}, {"notes_url", notesUrl}});
 				notesUrls.emplace_back(JsonEncode(data));
 
 				if (runtimeUpdate) {
-					AddObjectDataToRuntimeUpdates(runtimeUpdates, notesUrls.at(notesUrls.size() - 2u), m_PrefixConfigObject + "notes:url", data);
+					AddObjectDataToRuntimeUpdates(runtimeUpdates, id, m_PrefixConfigObject + "notes:url", data);
 				}
 			}
 		}
@@ -721,12 +729,12 @@ void IcingaDB::InsertObjectDependencies(const ConfigObject::Ptr& object, const S
 			auto id (HashValue(new Array({m_EnvironmentId, iconImage})));
 
 			if (runtimeUpdate || m_DumpedGlobals.IconImage.IsNew(id)) {
-				iconImages.emplace_back(std::move(id));
+				iconImages.emplace_back(id);
 				Dictionary::Ptr data = new Dictionary({{"environment_id", m_EnvironmentId}, {"icon_image", iconImage}});
 				iconImages.emplace_back(JsonEncode(data));
 
 				if (runtimeUpdate) {
-					AddObjectDataToRuntimeUpdates(runtimeUpdates, iconImages.at(iconImages.size() - 2u), m_PrefixConfigObject + "icon:image", data);
+					AddObjectDataToRuntimeUpdates(runtimeUpdates, id, m_PrefixConfigObject + "icon:image", data);
 				}
 			}
 		}

lib/icingadb/redisconnection.cpp

@@ -30,6 +30,15 @@ namespace asio = boost::asio;
 
 boost::regex RedisConnection::m_ErrAuth ("\\AERR AUTH ");
 
+RedisConnection::QueryArg::operator std::string_view() const
+{
+	if (auto str (std::get_if<String>(&m_Data)); str) {
+		return *str;
+	}
+
+	return std::get<std::string_view>(m_Data);
+}
+
 RedisConnection::RedisConnection(const String& host, int port, const String& path, const String& username, const String& password, int db,
 	bool useTls, bool insecure, const String& certPath, const String& keyPath, const String& caPath, const String& crlPath,
 	const String& tlsProtocolmin, const String& cipherList, double connectTimeout, DebugInfo di, const RedisConnection::Ptr& parent)
@@ -99,10 +108,12 @@ void LogQuery(RedisConnection::Query& query, Log& msg)
 			break;
 		}
 
-		if (arg.GetLength() > 64) {
-			msg << " '" << arg.SubStr(0, 61) << "...'";
+		std::string_view sv (arg);
+
+		if (sv.length() > 64) {
+			msg << " '" << sv.substr(0, 61) << "...'";
 		} else {
-			msg << " '" << arg << '\'';
+			msg << " '" << sv << '\'';
 		}
 	}
 }

lib/icingadb/redisconnection.hpp

@@ -38,7 +38,9 @@
 #include <queue>
 #include <set>
 #include <stdexcept>
+#include <string_view>
 #include <utility>
+#include <variant>
 #include <vector>
 
 namespace icinga
@@ -53,7 +55,39 @@ namespace icinga
 	public:
 		DECLARE_PTR_TYPEDEFS(RedisConnection);
 
-		typedef std::vector<String> Query;
+		/**
+		 * A Redis query argument. Either owned String, borrowed std::string_view or hardcoded const char[].
+		 * Allows mixing these types in a single query transparently, not requiring any conversions.
+		 *
+		 * @ingroup icingadb
+		 */
+		class QueryArg
+		{
+		public:
+			explicit QueryArg(std::string_view data) : m_Data(data)
+			{
+			}
+
+			QueryArg(String data): m_Data(std::move(data))
+			{
+			}
+
+			QueryArg(const char* data) : m_Data(std::string_view(data))
+			{
+			}
+
+			explicit operator std::string_view() const;
+
+			std::variant<std::string_view, String>& GetData()
+			{
+				return m_Data;
+			}
+
+		private:
+			std::variant<std::string_view, String> m_Data;
+		};
+
+		typedef std::vector<QueryArg> Query;
 		typedef std::vector<Query> Queries;
 		typedef Value Reply;
 		typedef std::vector<Reply> Replies;
@@ -667,7 +701,9 @@ void RedisConnection::WriteRESP(AsyncWriteStream& stream, const Query& query, bo
 	msg << "*" << query.size() << "\r\n";
 
 	for (auto& arg : query) {
-		msg << "$" << arg.GetLength() << "\r\n" << arg << "\r\n";
+		std::string_view sv (arg);
+
+		msg << "$" << sv.length() << "\r\n" << sv << "\r\n";
 	}
 
 	asio::async_write(stream, writeBuffer, yc);