From 2997155f5645d91bee841d30202ae764909aa017 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Thu, 30 Apr 2020 22:02:41 +0200 Subject: [PATCH 1/3] Fix spacing of typing notifications --- resources/qml/TimelineView.qml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/resources/qml/TimelineView.qml b/resources/qml/TimelineView.qml index c42cd6e9..7df2ed0b 100644 --- a/resources/qml/TimelineView.qml +++ b/resources/qml/TimelineView.qml @@ -17,6 +17,10 @@ Page { palette: colors + FontMetrics { + id: fontMetrics + } + Settings { id: settings category: "user" @@ -116,10 +120,10 @@ Page { boundsBehavior: Flickable.StopAtBounds - ScrollHelper { - flickable: parent - anchors.fill: parent - } + ScrollHelper { + flickable: parent + anchors.fill: parent + } Shortcut { sequence: StandardKey.MoveToPreviousPage @@ -256,7 +260,7 @@ Page { Rectangle { id: chatFooter - height: Math.max(16, footerContent.height) + height: Math.max(fontMetrics.height * 1.2, footerContent.height) anchors.left: parent.left anchors.right: parent.right anchors.bottom: parent.bottom @@ -326,8 +330,4 @@ Page { } } } - - FontMetrics { - id: fontMetrics - } } From e6fcccc8bde60c84157e0a3adbae01b87c621b7a Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Thu, 30 Apr 2020 22:42:56 +0200 Subject: [PATCH 2/3] Don't store pending receipts in cache We don't get notified for every message. Sometimes we only get a read receipt for the newest message, which means old read receipts accumulate in the database. This least to some considerable CPU overhead, when checking if the timeline should be notified for new read receipts. Instead just always notify, since that has far less overhead in the worst case and doesn't need complicated cache cleanup. The old pending_receipts db is not removed for now. It should still have minimal storage overhead and we don't have a good mechanism for cache format upgrades atm. --- src/Cache.cpp | 147 ++------------------------------- src/Cache.h | 15 ---- src/Cache_p.h | 12 --- src/timeline/TimelineModel.cpp | 3 - 4 files changed, 7 insertions(+), 170 deletions(-) diff --git a/src/Cache.cpp b/src/Cache.cpp index bdca76f2..eb5c93aa 100644 --- a/src/Cache.cpp +++ b/src/Cache.cpp @@ -62,6 +62,7 @@ constexpr auto SYNC_STATE_DB("sync_state"); //! Read receipts per room/event. constexpr auto READ_RECEIPTS_DB("read_receipts"); constexpr auto NOTIFICATIONS_DB("sent_notifications"); +//! TODO: delete pending_receipts database on old cache versions //! Encryption related databases. @@ -703,70 +704,6 @@ Cache::setCurrentFormat() txn.commit(); } -std::vector -Cache::pendingReceiptsEvents(lmdb::txn &txn, const std::string &room_id) -{ - auto db = getPendingReceiptsDb(txn); - - std::string key, unused; - std::vector pending; - - auto cursor = lmdb::cursor::open(txn, db); - while (cursor.get(key, unused, MDB_NEXT)) { - ReadReceiptKey receipt; - try { - receipt = json::parse(key); - } catch (const nlohmann::json::exception &e) { - nhlog::db()->warn("pendingReceiptsEvents: {}", e.what()); - continue; - } - - if (receipt.room_id == room_id) - pending.emplace_back(QString::fromStdString(receipt.event_id)); - } - - cursor.close(); - - return pending; -} - -void -Cache::removePendingReceipt(lmdb::txn &txn, const std::string &room_id, const std::string &event_id) -{ - auto db = getPendingReceiptsDb(txn); - - ReadReceiptKey receipt_key{event_id, room_id}; - auto key = json(receipt_key).dump(); - - try { - lmdb::dbi_del(txn, db, lmdb::val(key.data(), key.size()), nullptr); - } catch (const lmdb::error &e) { - nhlog::db()->critical("removePendingReceipt: {}", e.what()); - } -} - -void -Cache::addPendingReceipt(const QString &room_id, const QString &event_id) -{ - auto txn = lmdb::txn::begin(env_); - auto db = getPendingReceiptsDb(txn); - - ReadReceiptKey receipt_key{event_id.toStdString(), room_id.toStdString()}; - auto key = json(receipt_key).dump(); - std::string empty; - - try { - lmdb::dbi_put(txn, - db, - lmdb::val(key.data(), key.size()), - lmdb::val(empty.data(), empty.size())); - } catch (const lmdb::error &e) { - nhlog::db()->critical("addPendingReceipt: {}", e.what()); - } - - txn.commit(); -} - CachedReceipts Cache::readReceipts(const QString &event_id, const QString &room_id) { @@ -802,30 +739,6 @@ Cache::readReceipts(const QString &event_id, const QString &room_id) return receipts; } -std::vector -Cache::filterReadEvents(const QString &room_id, - const std::vector &event_ids, - const std::string &excluded_user) -{ - std::vector read_events; - - for (const auto &event : event_ids) { - auto receipts = readReceipts(event, room_id); - - if (receipts.size() == 0) - continue; - - if (receipts.size() == 1) { - if (receipts.begin()->second == excluded_user) - continue; - } - - read_events.emplace_back(event); - } - - return read_events; -} - void Cache::updateReadReceipt(lmdb::txn &txn, const std::string &room_id, const Receipts &receipts) { @@ -881,27 +794,6 @@ Cache::updateReadReceipt(lmdb::txn &txn, const std::string &room_id, const Recei } } -void -Cache::notifyForReadReceipts(const std::string &room_id) -{ - auto txn = lmdb::txn::begin(env_); - - QSettings settings; - auto local_user = settings.value("auth/user_id").toString(); - - auto matches = filterReadEvents(QString::fromStdString(room_id), - pendingReceiptsEvents(txn, room_id), - local_user.toStdString()); - - for (const auto &m : matches) - removePendingReceipt(txn, room_id, m.toStdString()); - - if (!matches.empty()) - emit newReadReceipts(QString::fromStdString(room_id), matches); - - txn.commit(); -} - void Cache::calculateRoomReadStatus() { @@ -1019,7 +911,12 @@ Cache::saveState(const mtx::responses::Sync &res) std::map readStatus; for (const auto &room : res.rooms.join) { - notifyForReadReceipts(room.first); + if (!room.second.ephemeral.receipts.empty()) { + std::vector receipts; + for (const auto &receipt : room.second.ephemeral.receipts) + receipts.push_back(QString::fromStdString(receipt.first)); + emit newReadReceipts(QString::fromStdString(room.first), receipts); + } readStatus.emplace(QString::fromStdString(room.first), calculateRoomReadStatus(room.first)); } @@ -2634,36 +2531,6 @@ readReceipts(const QString &event_id, const QString &room_id) return instance_->readReceipts(event_id, room_id); } -//! Filter the events that have at least one read receipt. -std::vector -filterReadEvents(const QString &room_id, - const std::vector &event_ids, - const std::string &excluded_user) -{ - return instance_->filterReadEvents(room_id, event_ids, excluded_user); -} -//! Add event for which we are expecting some read receipts. -void -addPendingReceipt(const QString &room_id, const QString &event_id) -{ - instance_->addPendingReceipt(room_id, event_id); -} -void -removePendingReceipt(lmdb::txn &txn, const std::string &room_id, const std::string &event_id) -{ - instance_->removePendingReceipt(txn, room_id, event_id); -} -void -notifyForReadReceipts(const std::string &room_id) -{ - instance_->notifyForReadReceipts(room_id); -} -std::vector -pendingReceiptsEvents(lmdb::txn &txn, const std::string &room_id) -{ - return instance_->pendingReceiptsEvents(txn, room_id); -} - QByteArray image(const QString &url) { diff --git a/src/Cache.h b/src/Cache.h index bb042ea9..99c63550 100644 --- a/src/Cache.h +++ b/src/Cache.h @@ -154,21 +154,6 @@ using UserReceipts = std::multimap UserReceipts readReceipts(const QString &event_id, const QString &room_id); -//! Filter the events that have at least one read receipt. -std::vector -filterReadEvents(const QString &room_id, - const std::vector &event_ids, - const std::string &excluded_user); -//! Add event for which we are expecting some read receipts. -void -addPendingReceipt(const QString &room_id, const QString &event_id); -void -removePendingReceipt(lmdb::txn &txn, const std::string &room_id, const std::string &event_id); -void -notifyForReadReceipts(const std::string &room_id); -std::vector -pendingReceiptsEvents(lmdb::txn &txn, const std::string &room_id); - QByteArray image(const QString &url); QByteArray diff --git a/src/Cache_p.h b/src/Cache_p.h index 6eae45a9..982099ea 100644 --- a/src/Cache_p.h +++ b/src/Cache_p.h @@ -137,18 +137,6 @@ public: using UserReceipts = std::multimap>; UserReceipts readReceipts(const QString &event_id, const QString &room_id); - //! Filter the events that have at least one read receipt. - std::vector filterReadEvents(const QString &room_id, - const std::vector &event_ids, - const std::string &excluded_user); - //! Add event for which we are expecting some read receipts. - void addPendingReceipt(const QString &room_id, const QString &event_id); - void removePendingReceipt(lmdb::txn &txn, - const std::string &room_id, - const std::string &event_id); - void notifyForReadReceipts(const std::string &room_id); - std::vector pendingReceiptsEvents(lmdb::txn &txn, const std::string &room_id); - QByteArray image(const QString &url) const; QByteArray image(lmdb::txn &txn, const std::string &url) const; void saveImage(const std::string &url, const std::string &data); diff --git a/src/timeline/TimelineModel.cpp b/src/timeline/TimelineModel.cpp index 97b95258..d68adf92 100644 --- a/src/timeline/TimelineModel.cpp +++ b/src/timeline/TimelineModel.cpp @@ -173,9 +173,6 @@ TimelineModel::TimelineModel(TimelineViewManager *manager, QString room_id, QObj // mark our messages as read readEvent(event_id.toStdString()); - // ask to be notified for read receipts - cache::addPendingReceipt(room_id_, event_id); - emit dataChanged(index(idx, 0), index(idx, 0)); if (pending.size() > 0) From 641a883bfdc65c7c9a86e799912e1aaf4dd63f05 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Thu, 30 Apr 2020 23:59:17 +0200 Subject: [PATCH 3/3] Optimize RoomList sorting Keep the almost sorted list around and sort by the raw timestamp value instead of doing the expensive toMSecSinceEpoch conversion. --- src/CacheStructs.h | 3 ++- src/RoomInfoListItem.cpp | 7 ++--- src/RoomList.cpp | 57 ++++++++++++++++++++-------------------- src/RoomList.h | 1 + src/Utils.cpp | 8 +++--- 5 files changed, 41 insertions(+), 35 deletions(-) diff --git a/src/CacheStructs.h b/src/CacheStructs.h index 2051afc8..ab7bbc71 100644 --- a/src/CacheStructs.h +++ b/src/CacheStructs.h @@ -39,7 +39,8 @@ struct DescInfo QString event_id; QString userid; QString body; - QString timestamp; + QString descriptiveTime; + uint64_t timestamp; QDateTime datetime; }; diff --git a/src/RoomInfoListItem.cpp b/src/RoomInfoListItem.cpp index 13c7b54d..ee8d532d 100644 --- a/src/RoomInfoListItem.cpp +++ b/src/RoomInfoListItem.cpp @@ -187,10 +187,11 @@ RoomInfoListItem::paintEvent(QPaintEvent *event) QFont tsFont; tsFont.setPointSizeF(tsFont.pointSizeF() * 0.9); #if QT_VERSION < QT_VERSION_CHECK(5, 11, 0) - const int msgStampWidth = QFontMetrics(tsFont).width(lastMsgInfo_.timestamp) + 4; + const int msgStampWidth = + QFontMetrics(tsFont).width(lastMsgInfo_.descriptiveTime) + 4; #else const int msgStampWidth = - QFontMetrics(tsFont).horizontalAdvance(lastMsgInfo_.timestamp) + 4; + QFontMetrics(tsFont).horizontalAdvance(lastMsgInfo_.descriptiveTime) + 4; #endif // We use the full width of the widget if there is no unread msg bubble. const int bottomLineWidthLimit = (unreadMsgCount_ > 0) ? msgStampWidth : 0; @@ -227,7 +228,7 @@ RoomInfoListItem::paintEvent(QPaintEvent *event) p.setFont(tsFont); p.drawText(QPoint(width() - wm.padding - msgStampWidth, top_y), - lastMsgInfo_.timestamp); + lastMsgInfo_.descriptiveTime); p.restore(); } else { int btnWidth = (width() - wm.iconSize - 6 * wm.padding) / 2; diff --git a/src/RoomList.cpp b/src/RoomList.cpp index 764fabb4..617710c4 100644 --- a/src/RoomList.cpp +++ b/src/RoomList.cpp @@ -82,7 +82,9 @@ RoomList::addRoom(const QString &room_id, const RoomInfo &info) MainWindow::instance()->openLeaveRoomDialog(room_id); }); - rooms_.emplace(room_id, QSharedPointer(room_item)); + QSharedPointer roomWidget(room_item); + rooms_.emplace(room_id, roomWidget); + rooms_sort_cache_.push_back(roomWidget); if (!info.avatar_url.empty()) updateAvatar(room_id, QString::fromStdString(info.avatar_url)); @@ -100,6 +102,14 @@ RoomList::updateAvatar(const QString &room_id, const QString &url) void RoomList::removeRoom(const QString &room_id, bool reset) { + auto roomIt = rooms_.find(room_id); + for (auto roomSortIt = rooms_sort_cache_.begin(); roomSortIt != rooms_sort_cache_.end(); + ++roomSortIt) { + if (roomIt->second == *roomSortIt) { + rooms_sort_cache_.erase(roomSortIt); + break; + } + } rooms_.erase(room_id); if (rooms_.empty() || !reset) @@ -336,7 +346,8 @@ RoomList::updateRoomDescription(const QString &roomid, const DescInfo &info) struct room_sort { - bool operator()(const RoomInfoListItem *a, const RoomInfoListItem *b) const + bool operator()(const QSharedPointer a, + const QSharedPointer b) const { // Sort by "importance" (i.e. invites before mentions before // notifs before new events before old events), then secondly @@ -351,12 +362,10 @@ struct room_sort // Now sort by recency // Zero if empty, otherwise the time that the event occured - const uint64_t a_recency = a->lastMessageInfo().userid.isEmpty() - ? 0 - : a->lastMessageInfo().datetime.toMSecsSinceEpoch(); - const uint64_t b_recency = b->lastMessageInfo().userid.isEmpty() - ? 0 - : b->lastMessageInfo().datetime.toMSecsSinceEpoch(); + const uint64_t a_recency = + a->lastMessageInfo().userid.isEmpty() ? 0 : a->lastMessageInfo().timestamp; + const uint64_t b_recency = + b->lastMessageInfo().userid.isEmpty() ? 0 : b->lastMessageInfo().timestamp; return a_recency > b_recency; } }; @@ -366,27 +375,17 @@ RoomList::sortRoomsByLastMessage() { isSortPending_ = false; - std::multiset times; + std::sort(begin(rooms_sort_cache_), end(rooms_sort_cache_), room_sort{}); - for (int ii = 0; ii < contentsLayout_->count(); ++ii) { - auto room = qobject_cast(contentsLayout_->itemAt(ii)->widget()); + int newIndex = 0; + for (const auto &roomWidget : rooms_sort_cache_) { + const auto currentIndex = contentsLayout_->indexOf(roomWidget.get()); - if (!room) - continue; - else - times.insert(room); - } - - for (auto it = times.cbegin(); it != times.cend(); ++it) { - const auto roomWidget = *it; - const auto currentIndex = contentsLayout_->indexOf(roomWidget); - const auto newIndex = std::distance(times.cbegin(), it); - - if (currentIndex == newIndex) - continue; - - contentsLayout_->removeWidget(roomWidget); - contentsLayout_->insertWidget(newIndex, roomWidget); + if (currentIndex != newIndex) { + contentsLayout_->removeWidget(roomWidget.get()); + contentsLayout_->insertWidget(newIndex, roomWidget.get()); + } + newIndex++; } } @@ -500,7 +499,9 @@ RoomList::addInvitedRoom(const QString &room_id, const RoomInfo &info) connect(room_item, &RoomInfoListItem::acceptInvite, this, &RoomList::acceptInvite); connect(room_item, &RoomInfoListItem::declineInvite, this, &RoomList::declineInvite); - rooms_.emplace(room_id, QSharedPointer(room_item)); + QSharedPointer roomWidget(room_item); + rooms_.emplace(room_id, roomWidget); + rooms_sort_cache_.push_back(roomWidget); updateAvatar(room_id, QString::fromStdString(info.avatar_url)); diff --git a/src/RoomList.h b/src/RoomList.h index a0151f92..d3470666 100644 --- a/src/RoomList.h +++ b/src/RoomList.h @@ -100,6 +100,7 @@ private: OverlayModal *joinRoomModal_; std::map> rooms_; + std::vector> rooms_sort_cache_; QString selectedRoom_; bool isSortPending_ = false; diff --git a/src/Utils.cpp b/src/Utils.cpp index f0a8d61b..7f11a8cd 100644 --- a/src/Utils.cpp +++ b/src/Utils.cpp @@ -41,6 +41,7 @@ createDescriptionInfo(const Event &event, const QString &localUser, const QStrin utils::messageDescription( username, QString::fromStdString(msg.content.body).trimmed(), sender == localUser), utils::descriptiveTime(ts), + msg.origin_server_ts, ts}; } @@ -184,9 +185,10 @@ utils::getMessageDescription(const TimelineEvent &event, info.userid = sender; info.body = QString(" %1").arg( messageDescription(username, "", sender == localUser)); - info.timestamp = utils::descriptiveTime(ts); - info.event_id = QString::fromStdString(msg->event_id); - info.datetime = ts; + info.timestamp = msg->origin_server_ts; + info.descriptiveTime = utils::descriptiveTime(ts); + info.event_id = QString::fromStdString(msg->event_id); + info.datetime = ts; return info; }