From: Manuel Nickschas Date: Sat, 15 Feb 2014 20:40:45 +0000 (+0100) Subject: DataStreamPeer: Optimize IrcUsersAndChannels init data X-Git-Tag: 0.10-beta1~22 X-Git-Url: https://git.quassel-irc.org/?p=quassel.git;a=commitdiff_plain;h=208ccb6d91ebb3c26a67c35c11411ba3ab27708a DataStreamPeer: Optimize IrcUsersAndChannels init data This is the only occasion where we change the actual message contents for the DataStream protocol, as opposed to the legacy protocol. However, it's worth it, because the init data for IrcUsers is the single biggest offender in terms of data usage during sync, and we were wasting lots of space by sending potentially thousands of identically-structured QVariantMaps (one per IrcUser), each of which contains an indentical set of key names. The same was true for IrcChannels in that same message, although the data set tends to be smaller for that one. We've now changed that by sending the init data for all the users and channels as lists of attributes, which are then reassembled into individual initData maps after receiving them. Our benchmarks show space savings around 56%, which can amount to several megabytes in the uncompressed case, provided you are in lots of busy channels. With compression it's less impressive, but we can still save a few hundred kilobytes. The legacy protocol gains a pair of translation methods for this special case, thus there are no changes in the legacy format. --- diff --git a/src/common/network.cpp b/src/common/network.cpp index 3fc7dff7..d18f7130 100644 --- a/src/common/network.cpp +++ b/src/common/network.cpp @@ -234,8 +234,12 @@ IrcUser *Network::newIrcUser(const QString &hostmask, const QVariantMap &initDat _ircUsers[nick] = ircuser; - SYNC_OTHER(addIrcUser, ARG(hostmask)) - // emit ircUserAdded(hostmask); + // This method will be called with a nick instead of hostmask by setInitIrcUsersAndChannels(). + // Not a problem because initData contains all we need; however, making sure here to get the real + // hostmask out of the IrcUser afterwards. + QString mask = ircuser->hostmask(); + SYNC_OTHER(addIrcUser, ARG(mask)); + // emit ircUserAdded(mask); emit ircUserAdded(ircuser); } @@ -774,27 +778,54 @@ QVariantMap Network::initSupports() const } +// There's potentially a lot of users and channels, so it makes sense to optimize the format of this. +// Rather than sending a thousand maps with identical keys, we convert this into one map containing lists +// where each list index corresponds to a particular IrcUser. This saves sending the key names a thousand times. +// Benchmarks have shown space savings of around 56%, resulting in saving several MBs worth of data on sync +// (without compression) with a decent amount of IrcUsers. QVariantMap Network::initIrcUsersAndChannels() const { QVariantMap usersAndChannels; - QVariantMap users; - QVariantMap channels; - - QHash::const_iterator userIter = _ircUsers.constBegin(); - QHash::const_iterator userIterEnd = _ircUsers.constEnd(); - while (userIter != userIterEnd) { - users[userIter.value()->hostmask()] = userIter.value()->toVariantMap(); - userIter++; + + if (_ircUsers.count()) { + QHash users; + QHash::const_iterator it = _ircUsers.begin(); + QHash::const_iterator end = _ircUsers.end(); + while (it != end) { + const QVariantMap &map = it.value()->toVariantMap(); + QVariantMap::const_iterator mapiter = map.begin(); + while (mapiter != map.end()) { + users[mapiter.key()] << mapiter.value(); + ++mapiter; + } + ++it; + } + // Can't have a container with a value type != QVariant in a QVariant :( + // However, working directly on a QVariantMap is awkward for appending, thus the detour via the hash above. + QVariantMap userMap; + foreach(const QString &key, users.keys()) + userMap[key] = users[key]; + usersAndChannels["Users"] = userMap; } - usersAndChannels["users"] = users; - QHash::const_iterator channelIter = _ircChannels.constBegin(); - QHash::const_iterator channelIterEnd = _ircChannels.constEnd(); - while (channelIter != channelIterEnd) { - channels[channelIter.value()->name()] = channelIter.value()->toVariantMap(); - channelIter++; + if (_ircChannels.count()) { + QHash channels; + QHash::const_iterator it = _ircChannels.begin(); + QHash::const_iterator end = _ircChannels.end(); + while (it != end) { + const QVariantMap &map = it.value()->toVariantMap(); + QVariantMap::const_iterator mapiter = map.begin(); + while (mapiter != map.end()) { + channels[mapiter.key()] << mapiter.value(); + ++mapiter; + } + ++it; + } + QVariantMap channelMap; + foreach(const QString &key, channels.keys()) + channelMap[key] = channels[key]; + usersAndChannels["Channels"] = channelMap; } - usersAndChannels["channels"] = channels; return usersAndChannels; } @@ -804,24 +835,49 @@ void Network::initSetIrcUsersAndChannels(const QVariantMap &usersAndChannels) { Q_ASSERT(proxy()); if (isInitialized()) { - qWarning() << "Network" << networkId() << "received init data for users and channels allthough there allready are known users or channels!"; + qWarning() << "Network" << networkId() << "received init data for users and channels although there already are known users or channels!"; return; } - QVariantMap users = usersAndChannels.value("users").toMap(); - QVariantMap::const_iterator userIter = users.constBegin(); - QVariantMap::const_iterator userIterEnd = users.constEnd(); - while (userIter != userIterEnd) { - newIrcUser(userIter.key(), userIter.value().toMap()); - userIter++; + // toMap() and toList() are cheap, so we can avoid copying to lists... + // However, we really have to make sure to never accidentally detach from the shared data! + + const QVariantMap &users = usersAndChannels["Users"].toMap(); + + // sanity check + int count = users["nick"].toList().count(); + foreach(const QString &key, users.keys()) { + if (users[key].toList().count() != count) { + qWarning() << "Received invalid usersAndChannels init data, sizes of attribute lists don't match!"; + return; + } + } + + // now create the individual IrcUsers + for(int i = 0; i < count; i++) { + QVariantMap map; + foreach(const QString &key, users.keys()) + map[key] = users[key].toList().at(i); + newIrcUser(map["nick"].toString(), map); // newIrcUser() properly handles the hostmask being just the nick } - QVariantMap channels = usersAndChannels.value("channels").toMap(); - QVariantMap::const_iterator channelIter = channels.constBegin(); - QVariantMap::const_iterator channelIterEnd = channels.constEnd(); - while (channelIter != channelIterEnd) { - newIrcChannel(channelIter.key(), channelIter.value().toMap()); - channelIter++; + // same thing for IrcChannels + const QVariantMap &channels = usersAndChannels["Channels"].toMap(); + + // sanity check + count = channels["name"].toList().count(); + foreach(const QString &key, channels.keys()) { + if (channels[key].toList().count() != count) { + qWarning() << "Received invalid usersAndChannels init data, sizes of attribute lists don't match!"; + return; + } + } + // now create the individual IrcChannels + for(int i = 0; i < count; i++) { + QVariantMap map; + foreach(const QString &key, channels.keys()) + map[key] = channels[key].toList().at(i); + newIrcChannel(map["name"].toString(), map); } } diff --git a/src/common/protocols/legacy/legacypeer.cpp b/src/common/protocols/legacy/legacypeer.cpp index e2890ccb..b292223e 100644 --- a/src/common/protocols/legacy/legacypeer.cpp +++ b/src/common/protocols/legacy/legacypeer.cpp @@ -437,6 +437,10 @@ void LegacyPeer::handlePackedFunc(const QVariant &packedFunc) QByteArray className = params[0].toByteArray(); QString objectName = params[1].toString(); QVariantMap initData = params[2].toMap(); + + // we need to special-case IrcUsersAndChannels here, since the format changed + if (className == "Network") + fromLegacyIrcUsersAndChannels(initData); handle(Protocol::InitData(className, objectName, initData)); break; } @@ -489,7 +493,14 @@ void LegacyPeer::dispatch(const Protocol::InitRequest &msg) void LegacyPeer::dispatch(const Protocol::InitData &msg) { - dispatchPackedFunc(QVariantList() << (qint16)InitData << msg.className << msg.objectName << msg.initData); + // We need to special-case IrcUsersAndChannels, as the format changed + if (msg.className == "Network") { + QVariantMap initData = msg.initData; + toLegacyIrcUsersAndChannels(initData); + dispatchPackedFunc(QVariantList() << (qint16)InitData << msg.className << msg.objectName << initData); + } + else + dispatchPackedFunc(QVariantList() << (qint16)InitData << msg.className << msg.objectName << msg.initData); } @@ -509,3 +520,72 @@ void LegacyPeer::dispatchPackedFunc(const QVariantList &packedFunc) { writeSocketData(QVariant(packedFunc)); } + + +// Handle the changed format for Network's initData +// cf. Network::initIrcUsersAndChannels() +void LegacyPeer::fromLegacyIrcUsersAndChannels(QVariantMap &initData) +{ + const QVariantMap &legacyMap = initData["IrcUsersAndChannels"].toMap(); + QVariantMap newMap; + + QHash users; + foreach(const QVariant &v, legacyMap["users"].toMap().values()) { + const QVariantMap &map = v.toMap(); + foreach(const QString &key, map.keys()) + users[key] << map[key]; + } + QVariantMap userMap; + foreach(const QString &key, users.keys()) + userMap[key] = users[key]; + newMap["Users"] = userMap; + + QHash channels; + foreach(const QVariant &v, legacyMap["channels"].toMap().values()) { + const QVariantMap &map = v.toMap(); + foreach(const QString &key, map.keys()) + channels[key] << map[key]; + } + QVariantMap channelMap; + foreach(const QString &key, channels.keys()) + channelMap[key] = channels[key]; + newMap["Channels"] = channelMap; + + initData["IrcUsersAndChannels"] = newMap; +} + + +void LegacyPeer::toLegacyIrcUsersAndChannels(QVariantMap &initData) +{ + const QVariantMap &usersAndChannels = initData["IrcUsersAndChannels"].toMap(); + QVariantMap legacyMap; + + // toMap() and toList() are cheap, so no need to copy to a hash + + QVariantMap userMap; + const QVariantMap &users = usersAndChannels["Users"].toMap(); + + int size = users["nick"].toList().size(); // we know this key exists + for(int i = 0; i < size; i++) { + QVariantMap map; + foreach(const QString &key, users.keys()) + map[key] = users[key].toList().at(i); + QString hostmask = QString("%1!%2@%3").arg(map["nick"].toString(), map["user"].toString(), map["host"].toString()); + userMap[hostmask.toLower()] = map; + } + legacyMap["users"] = userMap; + + QVariantMap channelMap; + const QVariantMap &channels = usersAndChannels["Channels"].toMap(); + + size = channels["name"].toList().size(); + for(int i = 0; i < size; i++) { + QVariantMap map; + foreach(const QString &key, channels.keys()) + map[key] = channels[key].toList().at(i); + channelMap[map["name"].toString().toLower()] = map; + } + legacyMap["channels"] = channelMap; + + initData["IrcUsersAndChannels"] = legacyMap; +} diff --git a/src/common/protocols/legacy/legacypeer.h b/src/common/protocols/legacy/legacypeer.h index 3bc93d31..5aeafcae 100644 --- a/src/common/protocols/legacy/legacypeer.h +++ b/src/common/protocols/legacy/legacypeer.h @@ -82,6 +82,9 @@ private: void handlePackedFunc(const QVariant &packedFunc); void dispatchPackedFunc(const QVariantList &packedFunc); + void toLegacyIrcUsersAndChannels(QVariantMap &initData); + void fromLegacyIrcUsersAndChannels(QVariantMap &initData); + QDataStream _stream; quint32 _blockSize; bool _useCompression;