From c1ec80ea61fa8408464f14ffd1608de07ea96966 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 28 Sep 2025 11:06:35 +0200 Subject: [PATCH 1/3] Event: editContentJson() and onContentChanged() onContentChanged() is necessary because EventTemplate stores event content as a C++ structure after converting it from JSON; while not all content JSON changes may lead to changes in that structure, there's no way to tell. Alternatively, we could get rid of storing the converted structure entirely - conversion usually doesn't take long anyway, if it repeats even another couple of times that should be fiiine?.. In return, we get savings from not doing conversions in bulk while loading events from syncs and backpagination. --- Quotient/events/event.h | 8 ++++++++ Quotient/events/roommessageevent.cpp | 5 ++--- Quotient/events/stateevent.h | 2 ++ 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/Quotient/events/event.h b/Quotient/events/event.h index fb5b3c23b..76f095632 100644 --- a/Quotient/events/event.h +++ b/Quotient/events/event.h @@ -338,6 +338,14 @@ class QUOTIENT_API Event { explicit Event(const QJsonObject& json); QJsonObject& editJson() { return _json; } + + virtual void onContentChanged() {} + void editContentJson(auto visitor) + { + editSubobject(_json, ContentKey, visitor); + onContentChanged(); + } + virtual void dumpTo(QDebug dbg) const; private: diff --git a/Quotient/events/roommessageevent.cpp b/Quotient/events/roommessageevent.cpp index e8e390bf9..bd7d157dd 100644 --- a/Quotient/events/roommessageevent.cpp +++ b/Quotient/events/roommessageevent.cpp @@ -316,9 +316,8 @@ QString RoomMessageEvent::fileNameToDownload() const void RoomMessageEvent::updateFileSourceInfo(const FileSourceInfo& fsi) { - editSubobject(editJson(), ContentKey, [&fsi](QJsonObject& contentJson) { - Quotient::fillJson(contentJson, { "url"_L1, "file"_L1 }, fsi); - }); + editContentJson( + [&fsi](QJsonObject &contentJson) { fillJson(contentJson, {"url"_L1, "file"_L1}, fsi); }); } QString rawMsgTypeForMimeType(const QMimeType& mimeType) diff --git a/Quotient/events/stateevent.h b/Quotient/events/stateevent.h index 778d3e73d..259e349c8 100644 --- a/Quotient/events/stateevent.h +++ b/Quotient/events/stateevent.h @@ -99,6 +99,8 @@ class EventTemplate QString prevSenderId() const { return _prev.senderId; } private: + void onContentChanged() override { _content = fromJson(Event::contentJson()); } + ContentT _content; Prev _prev; }; From 458cad00e81aa1b8750a18d201a5f823d56cd98b Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Mon, 29 Sep 2025 08:09:31 +0200 Subject: [PATCH 2/3] Don't store converted content in state events Instead, both current and previous content is now produced on the fly, on each content() and prevContent() call. This is obviously suboptimal in case of repeated content() calls but saves both time and space when most data from state events are not actually requested - which seems to be a common case, as clients rarely visualise each and every part of the state. When benchmarking on Quotest, and assuming the next commit (that optimises RoomMemberEvent, the most frequent state event of all) applied, the slowdown was about 2.5% and even that was within the deviation range. --- Quotient/events/simplestateevents.h | 21 +++++------ Quotient/events/stateevent.h | 54 ++++++++++------------------- 2 files changed, 30 insertions(+), 45 deletions(-) diff --git a/Quotient/events/simplestateevents.h b/Quotient/events/simplestateevents.h index 7dd800354..d2025e518 100644 --- a/Quotient/events/simplestateevents.h +++ b/Quotient/events/simplestateevents.h @@ -9,16 +9,17 @@ namespace Quotient { -#define DEFINE_SIMPLE_STATE_EVENT(Name_, TypeId_, ValueType_, GetterName_, JsonKey_) \ - constexpr inline auto Name_##Key = JsonKey_##_L1; \ - class QUOTIENT_API Name_ : public ::Quotient::KeylessStateEventBase< \ - Name_, EventContent::SingleKeyValue> { \ - public: \ - using value_type = ValueType_; \ - QUO_EVENT(Name_, TypeId_) \ - using KeylessStateEventBase::KeylessStateEventBase; \ - auto GetterName_() const { return content().value; } \ - }; \ +#define DEFINE_SIMPLE_STATE_EVENT(Name_, TypeId_, ValueType_, GetterName_, JsonKey_) \ + constexpr inline auto Name_##Key = JsonKey_##_L1; \ + class QUOTIENT_API Name_ : public ::Quotient::KeylessStateEventBase< \ + Name_, EventContent::SingleKeyValue> \ + { \ + public: \ + using value_type = ValueType_; \ + QUO_EVENT(Name_, TypeId_) \ + using KeylessStateEventBase::KeylessStateEventBase; \ + QUO_CONTENT_GETTER_X(ValueType_, GetterName_, Name_##Key) \ + }; \ // End of macro DEFINE_SIMPLE_STATE_EVENT(RoomNameEvent, "m.room.name", QString, name, "name") diff --git a/Quotient/events/stateevent.h b/Quotient/events/stateevent.h index 259e349c8..915c41a44 100644 --- a/Quotient/events/stateevent.h +++ b/Quotient/events/stateevent.h @@ -63,46 +63,30 @@ class EventTemplate public: using content_type = ContentT; - struct Prev { - explicit Prev() = default; - explicit Prev(const QJsonObject& unsignedJson) - : senderId(fromJson(unsignedJson["prev_sender"_L1])) - , content(fromJson>(unsignedJson[PrevContentKey])) - {} - - QString senderId; - std::optional content; - }; - - explicit EventTemplate(const QJsonObject& fullJson) - : StateEvent(fullJson) - , _content(fromJson(Event::contentJson())) - , _prev(unsignedJson()) - {} + explicit EventTemplate(const QJsonObject &fullJson) : StateEvent(fullJson) {} + template - explicit EventTemplate(const QString& stateKey, - ContentParamTs&&... contentParams) - : StateEvent(EventT::TypeId, stateKey) - , _content { std::forward(contentParams)... } - { - editJson().insert(ContentKey, toJson(_content)); - } + explicit EventTemplate(const QString &stateKey, ContentParamTs &&...contentParams) + : StateEvent(EventT::TypeId, stateKey, + toJson(ContentT{std::forward(contentParams)...})) + {} - const ContentT& content() const { return _content; } + ContentT content() const { return fromJson(Event::contentJson()); } - void editContent(auto&& visitor) + template VisitorT> + void editContent(VisitorT &&visitor) { - visitor(_content); - editJson()[ContentKey] = toJson(_content); + editContentJson([&visitor](QJsonObject &contentJson) mutable { + auto content = fromJson(contentJson); + std::invoke(std::forward(visitor), content); + contentJson = toJson(content); + }); } - const std::optional& prevContent() const { return _prev.content; } - QString prevSenderId() const { return _prev.senderId; } - -private: - void onContentChanged() override { _content = fromJson(Event::contentJson()); } - - ContentT _content; - Prev _prev; + std::optional prevContent() const + { + return unsignedPart>(PrevContentKey); + } + QString prevSenderId() const { return unsignedPart("prev_sender"_L1); } }; template From 8f7e262b8f14c4da6abe4964f50a546dea42d7b3 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Mon, 29 Sep 2025 09:26:03 +0200 Subject: [PATCH 3/3] RoomMemberEvent: don't deserialise the whole content in accessors --- Quotient/events/roommemberevent.h | 15 ++++++++++----- Quotient/room.cpp | 3 +-- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/Quotient/events/roommemberevent.h b/Quotient/events/roommemberevent.h index 9d4f31a4b..d5f59617f 100644 --- a/Quotient/events/roommemberevent.h +++ b/Quotient/events/roommemberevent.h @@ -36,12 +36,17 @@ class QUOTIENT_API RoomMemberEvent using KeyedStateEventBase::KeyedStateEventBase; - Membership membership() const { return content().membership; } + QUO_CONTENT_GETTER(Membership, membership) + // Membership membership() const { return content().membership; } QString userId() const { return stateKey(); } - bool isDirect() const { return content().isDirect; } - std::optional newDisplayName() const { return content().displayName; } - std::optional newAvatarUrl() const { return content().avatarUrl; } - QString reason() const { return content().reason; } + QUO_CONTENT_GETTER(bool, isDirect) + // bool isDirect() const { return content().isDirect; } + QUO_CONTENT_GETTER_X(std::optional, newDisplayName, "displayname"_L1) + // std::optional newDisplayName() const { return content().displayName; } + QUO_CONTENT_GETTER_X(std::optional, newAvatarUrl, "avatar_url"_L1) + // std::optional newAvatarUrl() const { return content().avatarUrl; } + QUO_CONTENT_GETTER(QString, reason) + // QString reason() const { return content().reason; } bool changesMembership() const; bool isBan() const; bool isUnban() const; diff --git a/Quotient/room.cpp b/Quotient/room.cpp index ed8340f72..c1819c042 100644 --- a/Quotient/room.cpp +++ b/Quotient/room.cpp @@ -3307,8 +3307,7 @@ Room::Change Room::Private::processStateEvent(const RoomEvent& curEvent, }, [this, oldEvent](const JoinRulesEvent& evt) { if (const auto* oldJRE = static_cast(oldEvent); - oldJRE && oldJRE->content().joinRule != evt.content().joinRule - ) { + oldJRE && oldJRE->joinRule() != evt.joinRule()) { emit q->joinRuleChanged(); } return Change::Other;