From 3d07f723c84d0cc806090525001a7ee5b4cd746f Mon Sep 17 00:00:00 2001 From: Joshua Goins Date: Thu, 12 Feb 2026 18:43:05 -0500 Subject: [PATCH] Make member list sorted a bit more efficient I don't have any hard numbers on what difference this makes, but it's definitely a positive improvement. I noticed and fixed a few issues that were made more glaring by recent changes in libQuotient: 1. Room::memberJoined is called during the historical loading or whatever, when we only need that *after* stuff is settled. 2. We really don't need to sort the room's members immediately - it's only relevant when UserListModel is used (and I think this was previous behavior?) So now its done lazily. 3. We do not want to call Room::effectivePowerLevel willy-nilly. It may become a more expensive lookup, and it's also varying levels of wasteful depending on which sorting algorithm the STL uses. It doesn't cost much for us to keep a temporary cache for the lambda function to use. --- src/libneochat/models/userlistmodel.cpp | 4 ++++ src/libneochat/neochatroom.cpp | 28 ++++++++++++++++++------- src/libneochat/neochatroom.h | 11 ++++++++-- 3 files changed, 34 insertions(+), 9 deletions(-) diff --git a/src/libneochat/models/userlistmodel.cpp b/src/libneochat/models/userlistmodel.cpp index 71226ea3c..ae349af9d 100644 --- a/src/libneochat/models/userlistmodel.cpp +++ b/src/libneochat/models/userlistmodel.cpp @@ -172,6 +172,10 @@ void UserListModel::refreshAllMembers() { beginResetModel(); if (m_currentRoom != nullptr) { + // Only sort members when this model needs to be refreshed. + if (m_currentRoom->sortedMemberIds().isEmpty()) { + m_currentRoom->sortAllMembers(); + } m_members = m_currentRoom->sortedMemberIds(); } else { m_members.clear(); diff --git a/src/libneochat/neochatroom.cpp b/src/libneochat/neochatroom.cpp index 91f16c3c9..8e8381d0b 100644 --- a/src/libneochat/neochatroom.cpp +++ b/src/libneochat/neochatroom.cpp @@ -175,9 +175,16 @@ NeoChatRoom::NeoChatRoom(Connection *connection, QString roomId, JoinState joinS connect(neochatconnection, &NeoChatConnection::globalUrlPreviewEnabledChanged, this, &NeoChatRoom::urlPreviewEnabledChanged); connect(this, &Room::fullyReadMarkerMoved, this, &NeoChatRoom::invalidateLastUnreadHighlightId); - // Wait until the initial member list is available before sorting - connect(this, &Room::memberListChanged, this, &NeoChatRoom::refreshAllMembers, Qt::SingleShotConnection); - connect(this, &Room::memberJoined, this, &NeoChatRoom::insertMemberSorted); + // This may look weird, but this is actually for performance. + // We only want to listen to new member joining *when* the initial member list was loaded. + connect( + this, + &Room::memberListChanged, + this, + [this] { + connect(this, &Room::memberJoined, this, &NeoChatRoom::insertMemberSorted); + }, + Qt::SingleShotConnection); } bool NeoChatRoom::visible() const @@ -1917,14 +1924,21 @@ void NeoChatRoom::invalidateLastUnreadHighlightId(const QString &fromEventId, co } } -void NeoChatRoom::refreshAllMembers() +void NeoChatRoom::sortAllMembers() { m_sortedMemberIds = memberIds(); + // Build up a temporary cache, because we may be checking the same member over and over while sorting. + QHash effectivePowerLevels; + effectivePowerLevels.reserve(m_sortedMemberIds.size()); + for (const auto &member : m_sortedMemberIds) { + effectivePowerLevels[member] = memberEffectivePowerLevel(member); + } + MemberSorter sorter; - std::ranges::sort(m_sortedMemberIds, [this, &sorter](const auto &left, const auto &right) { - const auto leftPl = memberEffectivePowerLevel(left); - const auto rightPl = memberEffectivePowerLevel(right); + std::ranges::sort(m_sortedMemberIds, [&sorter, &effectivePowerLevels](const auto &left, const auto &right) { + const auto leftPl = effectivePowerLevels[left]; + const auto rightPl = effectivePowerLevels[right]; if (leftPl > rightPl) { return true; } diff --git a/src/libneochat/neochatroom.h b/src/libneochat/neochatroom.h index e03afcdf1..5015a9c14 100644 --- a/src/libneochat/neochatroom.h +++ b/src/libneochat/neochatroom.h @@ -683,6 +683,8 @@ public: /** * @return List of members in this room, sorted by power level and then by name. + * + * This list is only populated after sortAllMembers() is called. */ QList sortedMemberIds() const; @@ -721,6 +723,13 @@ public: */ Q_INVOKABLE void clearSelectedMessages(); + /** + * @brief Sort all members based on their display name, and power level. + * + * @note This is a very expensive operation, and should only be done when truly needed. + */ + void sortAllMembers(); + private: bool m_visible = false; @@ -767,8 +776,6 @@ private Q_SLOTS: void invalidateLastUnreadHighlightId(const QString &fromEventId, const QString &toEventId); - void refreshAllMembers(); - void insertMemberSorted(Quotient::RoomMember member); Q_SIGNALS: