From f4463dd17f6c3c1431e42166ee34cfc276824aff Mon Sep 17 00:00:00 2001 From: jolavillette Date: Sat, 11 Jul 2026 16:29:35 +0200 Subject: [PATCH 01/13] GxsForumModel: never freeze the UI on "mark all as read" recursSetMsgReadStatus() used to spawn one detached std::thread per changed post (each blocking up to 5s in markRead) and emit one dataChanged() per post. On a forum with thousands of unread posts this created thousands of threads and signals driven from the GUI thread, freezing (and sometimes crashing) the UI - the reported symptom being an hour-long hourglass on an 8000-post forum. Now the recursion only collects the affected message ids and updates the in-memory model; setMsgReadStatus() then issues a single background batch call (rsGxsForums->markRead(group, ids, read)) and a single view refresh. The GUI thread does O(n) in-memory work plus one thread and one signal, so it stays responsive no matter how large the forum is. Requires the matching libretroshare bulk markRead() overload. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/gui/gxsforums/GxsForumModel.cpp | 53 ++++++++++--------- .../src/gui/gxsforums/GxsForumModel.h | 2 +- 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp b/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp index 70151384c..6434cdfa5 100644 --- a/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp +++ b/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp @@ -871,19 +871,34 @@ void RsGxsForumModel::setMsgReadStatus(const QModelIndex& i,bool read_status,boo if(!convertRefPointerToTabEntry(ref,entry) || entry >= mPosts.size()) return ; + // Collect the posts whose read status actually changes and update the + // in-memory model right away, but do NOT touch the backend or the view once + // per message: doing so used to spawn one detached thread AND emit one + // dataChanged() per post, which froze the UI for a very long time (and could + // crash) when marking a large forum (thousands of posts) as read. + std::vector changed_msgs; + recursSetMsgReadStatus(entry,read_status,with_children,changed_msgs) ; + bool has_unread_below, has_read_below; - recursSetMsgReadStatus(entry,read_status,with_children) ; recursUpdateReadStatusAndTimes(0,has_unread_below,has_read_below); - // also emit dataChanged() for parents since they need to re-draw + // Persist every change in a single background batch. This never runs on the + // GUI thread and spawns exactly one thread regardless of how many posts are + // affected, so the UI stays responsive even on very large forums. + if(!changed_msgs.empty()) + RsThread::async( [grpId=mForumGroup.mMeta.mGroupId,changed_msgs,read_status]() + { + rsGxsForums->markRead(grpId, changed_msgs, read_status); + }); - for(QModelIndex j = i.parent(); j.isValid(); j = j.parent()) - { - emit dataChanged(j, j.sibling(j.row(), COLUMN_THREAD_NB_COLUMNS - 1)); - } + // A single refresh of the whole view instead of one dataChanged() per post. + if(mTreeMode == TREE_MODE_FLAT) + emit dataChanged(createIndex(0,0,(void*)NULL), createIndex(mPosts.size(),COLUMN_THREAD_NB_COLUMNS-1,(void*)NULL)); + else + emit dataChanged(createIndex(0,0,(void*)NULL), createIndex(mPosts[0].mChildren.size(),COLUMN_THREAD_NB_COLUMNS-1,(void*)NULL)); } -void RsGxsForumModel::recursSetMsgReadStatus(ForumModelIndex i,bool read_status,bool with_children) +void RsGxsForumModel::recursSetMsgReadStatus(ForumModelIndex i,bool read_status,bool with_children,std::vector& changed_msgs) { uint32_t newStatus = (read_status ? mPosts[i].mMsgStatus & ~static_cast(GXS_SERV::GXS_MSG_STATUS_GUI_UNREAD) : mPosts[i].mMsgStatus | static_cast(GXS_SERV::GXS_MSG_STATUS_GUI_UNREAD)); @@ -897,33 +912,21 @@ void RsGxsForumModel::recursSetMsgReadStatus(ForumModelIndex i,bool read_status, //Don't recurs post versions as this should be done before, if no change. auto s = getPostVersions(mPosts[i].mMsgId) ; + // Just collect the affected message ids here. The backend is updated + // once, in a single batched call issued by setMsgReadStatus(), instead + // of one detached thread (and one dataChanged()) per message. if(!s.empty()) for(auto it(s.begin());it!=s.end();++it) - { - RsThread::async( [grpId=mForumGroup.mMeta.mGroupId,msgId=it->second,original_msg_id=mPosts[i].mMsgId,read_status]() - { - rsGxsForums->markRead(std::make_pair( grpId, msgId ), read_status); - std::cerr << "Setting version " << msgId << " of post " << original_msg_id << " as read." << std::endl; - }); - } + changed_msgs.push_back(it->second); else - RsThread::async( [grpId=mForumGroup.mMeta.mGroupId,original_msg_id=mPosts[i].mMsgId,read_status]() - { - rsGxsForums->markRead(std::make_pair( grpId, original_msg_id), read_status); - }); - - void *ref ; - convertTabEntryToRefPointer(i,ref); // we dont use i+1 here because i is not a row, but an index in the mPosts tab - - QModelIndex itemIndex = (mTreeMode == TREE_MODE_FLAT)?createIndex(i - 1, 0, ref):createIndex(mPosts[i].prow,0,ref); - emit dataChanged(itemIndex, itemIndex.sibling(itemIndex.row(), COLUMN_THREAD_NB_COLUMNS - 1)); + changed_msgs.push_back(mPosts[i].mMsgId); } if(!with_children) return; for(uint32_t j=0;j& messages); void recursUpdateReadStatusAndTimes(ForumModelIndex i,bool& has_unread_below,bool& has_read_below); uint32_t recursUpdateFilterStatus(ForumModelIndex i,int column,const QStringList& strings); - void recursSetMsgReadStatus(ForumModelIndex i,bool read_status,bool with_children); + void recursSetMsgReadStatus(ForumModelIndex i,bool read_status,bool with_children,std::vector& changed_msgs); static void generateMissingItem(const RsGxsMessageId &msgId,ForumModelPostEntry& entry); static ForumModelIndex addEntry(std::vector& posts,const ForumModelPostEntry& entry,ForumModelIndex parent); From e5ed16c805f60216e1d974750f56657d9bcab979 Mon Sep 17 00:00:00 2001 From: jolavillette Date: Sun, 12 Jul 2026 00:03:22 +0200 Subject: [PATCH 02/13] Channels/Posted models: never freeze the UI on "mark all as read" GxsChannelPostsModel and PostedPostsModel spawned one detached std::thread (and got one event) per post on "mark all as read/unread", freezing the UI on large channels/boards - the same problem just fixed for forums, and worse here given the much larger channel/board databases. Both now collect the affected message ids and issue a single background batch call (setMessageReadStatus / setPostReadStatus with a vector) plus a single view refresh. PostedPostsModel additionally updates its local model up front (it previously relied entirely on the per-message events). Requires the matching libretroshare bulk overloads. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/gui/Posted/PostedPostsModel.cpp | 32 +++++++++++++++---- .../gui/gxschannels/GxsChannelPostsModel.cpp | 17 +++++++--- 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/retroshare-gui/src/gui/Posted/PostedPostsModel.cpp b/retroshare-gui/src/gui/Posted/PostedPostsModel.cpp index 6809379d3..cd014168a 100644 --- a/retroshare-gui/src/gui/Posted/PostedPostsModel.cpp +++ b/retroshare-gui/src/gui/Posted/PostedPostsModel.cpp @@ -737,20 +737,38 @@ void RsPostedPostsModel::createPostsArray(std::vector& posts) void RsPostedPostsModel::setAllMsgReadStatus(bool read) { - // make a temporary listof pairs + // Collect the posts whose status actually changes, persist them all in a + // single background batch (one thread and one event for the whole set, + // instead of one detached thread + one event per post, which froze the UI + // on large boards), then update the local model and refresh once. - std::list pairs; + std::vector msgIds; + msgIds.reserve(mPosts.size()); for(uint32_t i=0;isetPostReadStatus(p,read); + rsPosted->setPostReadStatus(boardId, msgIds, read); } ); + + // Update the local model immediately, since we don't catch the resulting + // event later (that would reload the posts). + + for(uint32_t i=0;i msgIds; + msgIds.reserve(pairs.size()); + for(const RsGxsGrpMsgIdPair& p : pairs) + msgIds.push_back(p.second); + + if(!msgIds.empty()) + RsThread::async([channelId=mChannelGroup.mMeta.mGroupId, msgIds, read_status]() { - if(!rsGxsChannels->setMessageReadStatus(p,read_status)) - RsErr() << "setAllMsgReadStatus: failed to change status of msg " << p.first << " in group " << p.second << " to status " << read_status << std::endl; + if(!rsGxsChannels->setMessageReadStatus(channelId, msgIds, read_status)) + RsErr() << "setAllMsgReadStatus: failed to change status of " << msgIds.size() << " messages in channel " << channelId << std::endl; }); // 3 - update the local model data, since we don't catch the READ_STATUS_CHANGED event later, to avoid re-loading the msg. From 2816dfd9f69871f3c5e2e18a44637015babdae33 Mon Sep 17 00:00:00 2001 From: jolavillette Date: Tue, 14 Jul 2026 10:37:12 +0200 Subject: [PATCH 03/13] gui(forums): fix slow post navigation by restricting dataChanged refresh to affected post and parents --- .../src/gui/gxsforums/GxsForumModel.cpp | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp b/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp index 6434cdfa5..db559496a 100644 --- a/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp +++ b/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp @@ -891,11 +891,23 @@ void RsGxsForumModel::setMsgReadStatus(const QModelIndex& i,bool read_status,boo rsGxsForums->markRead(grpId, changed_msgs, read_status); }); - // A single refresh of the whole view instead of one dataChanged() per post. - if(mTreeMode == TREE_MODE_FLAT) - emit dataChanged(createIndex(0,0,(void*)NULL), createIndex(mPosts.size(),COLUMN_THREAD_NB_COLUMNS-1,(void*)NULL)); + if (with_children || changed_msgs.size() > 5) + { + // A single refresh of the whole view instead of one dataChanged() per post. + if(mTreeMode == TREE_MODE_FLAT) + emit dataChanged(createIndex(0,0,(void*)NULL), createIndex(mPosts.size(),COLUMN_THREAD_NB_COLUMNS-1,(void*)NULL)); + else + emit dataChanged(createIndex(0,0,(void*)NULL), createIndex(mPosts[0].mChildren.size(),COLUMN_THREAD_NB_COLUMNS-1,(void*)NULL)); + } else - emit dataChanged(createIndex(0,0,(void*)NULL), createIndex(mPosts[0].mChildren.size(),COLUMN_THREAD_NB_COLUMNS-1,(void*)NULL)); + { + // Emit dataChanged only for the changed message and its parents + emit dataChanged(i, i.sibling(i.row(), COLUMN_THREAD_NB_COLUMNS - 1)); + for(QModelIndex j = i.parent(); j.isValid(); j = j.parent()) + { + emit dataChanged(j, j.sibling(j.row(), COLUMN_THREAD_NB_COLUMNS - 1)); + } + } } void RsGxsForumModel::recursSetMsgReadStatus(ForumModelIndex i,bool read_status,bool with_children,std::vector& changed_msgs) From 8959b4508772039b387a9ab6852fb17db400f4db Mon Sep 17 00:00:00 2001 From: jolavillette Date: Fri, 17 Jul 2026 22:41:11 +0200 Subject: [PATCH 04/13] gui(forums): keep single-post read latency low via unitary markRead Routing every read through the batched markRead(grpId, vector, read) also routed single interactive reads (selecting/opening one post) through its waitToken(): the READ_STATUS_CHANGED event, and thus the forum-list unread counter refresh, was only emitted once the DB write completed, adding ~100ms plus one GXS tick of latency versus the previous behaviour. Send a single changed message through the per-message markRead(pair), which emits the event immediately, and keep the batched call only for larger sets (mark-all, with-children, versioned posts) where deferring the single event until completion is what avoids the freeze. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/gui/gxsforums/GxsForumModel.cpp | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp b/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp index db559496a..ef19bcd8d 100644 --- a/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp +++ b/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp @@ -882,10 +882,20 @@ void RsGxsForumModel::setMsgReadStatus(const QModelIndex& i,bool read_status,boo bool has_unread_below, has_read_below; recursUpdateReadStatusAndTimes(0,has_unread_below,has_read_below); - // Persist every change in a single background batch. This never runs on the - // GUI thread and spawns exactly one thread regardless of how many posts are - // affected, so the UI stays responsive even on very large forums. - if(!changed_msgs.empty()) + // Persist the change(s) in the background so the GUI thread never blocks. + // A single interactive read (the common case: selecting/opening one post) + // goes through the per-message markRead(), which emits READ_STATUS_CHANGED + // right away, so the unread counters refresh without the extra latency added + // by the batch path (which only emits its event once waitToken() returns). + // Larger operations (mark-all, with-children, versioned posts) use the + // batched call, which persists everything in a single transaction and emits + // a single event when the whole set is done. + if(changed_msgs.size() == 1) + RsThread::async( [grpId=mForumGroup.mMeta.mGroupId,msgId=changed_msgs[0],read_status]() + { + rsGxsForums->markRead(std::make_pair(grpId, msgId), read_status); + }); + else if(!changed_msgs.empty()) RsThread::async( [grpId=mForumGroup.mMeta.mGroupId,changed_msgs,read_status]() { rsGxsForums->markRead(grpId, changed_msgs, read_status); From 78ff5d66e6cef0a27b8f638b801be31b9be17fd7 Mon Sep 17 00:00:00 2001 From: jolavillette Date: Fri, 24 Jul 2026 20:39:49 +0200 Subject: [PATCH 05/13] GxsForumModel: fix malformed model reset dropping freshly-arrived posts setPosts() called beginResetModel()/endResetModel() *before* swapping mPosts, so endResetModel() fired while the model still exposed the previous hierarchy, then emitted an extra beginInsertRows()/endInsertRows() on top of the reset. That double-signalling left the view/proxy row->source mapping stale, so a post that had just arrived could map to an invalid source index: selecting it neither displayed its content nor cleared its unread (bold) state. Bracket the actual data swap with beginResetModel()/endResetModel() and drop the bogus row-insertion signal; a full reset already tells the views to re-query everything. Same anti-pattern as the channels grid-empty-on-new-post fix. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/gui/gxsforums/GxsForumModel.cpp | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp b/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp index ef19bcd8d..966d3094f 100644 --- a/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp +++ b/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp @@ -759,8 +759,15 @@ void RsGxsForumModel::setPosts(const RsGxsForumGroup& group, const std::vectorsource mapping. + // The former code also emitted a bogus beginInsertRows()/endInsertRows() on top + // of the reset, double-counting rows and corrupting that mapping — which made a + // freshly arrived post map to an invalid source index (post not shown, title + // left bold when selected). A single reset already tells the views to re-query + // everything, so no explicit row-insertion signal is needed. beginResetModel(); - endResetModel(); mForumGroup = group; mPosts = posts; @@ -783,17 +790,7 @@ void RsGxsForumModel::setPosts(const RsGxsForumGroup& group, const std::vector0) - { - beginInsertRows(QModelIndex(),0,count-1); - endInsertRows(); - } + endResetModel(); postMods(); emit forumLoaded(); From 839293f84d1aa137f0dd9413ceb1564fe276604c Mon Sep 17 00:00:00 2001 From: jolavillette Date: Fri, 24 Jul 2026 20:40:13 +0200 Subject: [PATCH 06/13] gui(forums): coalesce full-forum reloads to stop sync-burst UI freezes handleEvent_main_thread() ran a full updateDisplay(true) (updateForum -> getForumPostsHierarchy -> setPosts full reset -> proxy re-sort/re-filter -> tree rebuild) for every incoming GXS event. A sync batch delivers many NEW_MESSAGE events back to back, so the forum was reloaded once per post, freezing the UI for seconds and resetting the model out from under the user's current selection (and racing the async read-status persistence, making a just-read post pop back to bold). Route those events through a single-shot QTimer (300 ms) that is restarted on each event, so a whole burst collapses into one reload once the events settle. An explicit reload (forum switch) cancels any pending deferred one. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../gui/gxsforums/GxsForumThreadWidget.cpp | 23 ++++++++++++++++++- .../src/gui/gxsforums/GxsForumThreadWidget.h | 10 ++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp index f55bbb0c0..a7de911c9 100644 --- a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp +++ b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include "util/qtthreadsutils.h" #include "util/misc.h" @@ -374,6 +375,12 @@ GxsForumThreadWidget::GxsForumThreadWidget(const RsGxsGroupId &forumId, QWidget ui->threadTreeWidget->enableColumnCustomize(true); #endif + // Single-shot timer used to coalesce the full-forum reloads requested by + // incoming GXS events (see scheduleForumReload()). + mDeferredReloadTimer = new QTimer(this); + mDeferredReloadTimer->setSingleShot(true); + connect(mDeferredReloadTimer, &QTimer::timeout, this, [this]() { updateDisplay(true); }); + mEventHandlerId = 0; // Needs to be asynced because this function is called by another thread! rsEvents->registerEventsHandler( @@ -402,7 +409,7 @@ void GxsForumThreadWidget::handleEvent_main_thread(std::shared_ptrmForumGroupId == mForumGroup.mMeta.mGroupId) - updateDisplay(true); + scheduleForumReload(); break; case RsForumEventCode::SUBSCRIBE_STATUS_CHANGED: @@ -424,6 +431,16 @@ void GxsForumThreadWidget::handleEvent_main_thread(std::shared_ptrstart(300); +} + void GxsForumThreadWidget::showForumInfo() { mThreadId.clear(); @@ -596,6 +613,10 @@ void GxsForumThreadWidget::updateDisplay(bool complete) } if(complete) // need to update the group data, reload the messages etc. { + // We are reloading now, so drop any reload still pending in the coalescing + // timer (e.g. queued by events for a forum we just switched away from). + mDeferredReloadTimer->stop(); + saveExpandedItems(mSavedExpandedMessages); if(groupId() != mThreadModel->currentGroupId()) diff --git a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.h b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.h index 8e6b1c01b..130f70bd0 100644 --- a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.h +++ b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.h @@ -29,6 +29,7 @@ #include "util/FontSizeHandler.h" class QSortFilterProxyModel; +class QTimer; class QTreeWidgetItem; class RSTreeWidgetItemCompareRole; class GxsForumsFillThread; @@ -197,6 +198,13 @@ private: void handleEvent_main_thread(std::shared_ptr event); + // Coalesce the full-forum reloads triggered by incoming GXS events: a sync + // burst delivers many NEW_MESSAGE events in a row, and reloading the whole + // forum for each one froze the UI for seconds and reset the model out from + // under the user's selection. Restart a single-shot timer instead so a burst + // results in one reload once the events settle. + void scheduleForumReload(); + private: void setForumDescriptionLoading(); void clearForumDescription(); @@ -239,6 +247,8 @@ private: Ui::GxsForumThreadWidget *ui; RsEventsHandlerId_t mEventHandlerId; + + QTimer *mDeferredReloadTimer; }; #endif // GXSFORUMTHREADWIDGET_H From 3860f76dc10943fbf729b586be7dcce594620a87 Mon Sep 17 00:00:00 2001 From: jolavillette Date: Sat, 25 Jul 2026 08:40:05 +0200 Subject: [PATCH 07/13] gui(forums): create reload timer before setGroupId (fix ctor SIGSEGV) The coalescing timer was allocated near the end of the constructor, but setGroupId(forumId) is called earlier and synchronously reaches groupIdChanged() -> updateDisplay(true) -> mDeferredReloadTimer->stop(), dereferencing the still-uninitialised pointer and crashing the moment a forum was opened. Allocate the timer right after setupUi(), before setGroupId(). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/gui/gxsforums/GxsForumThreadWidget.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp index a7de911c9..2e75199cf 100644 --- a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp +++ b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp @@ -254,6 +254,13 @@ GxsForumThreadWidget::GxsForumThreadWidget(const RsGxsGroupId &forumId, QWidget { ui->setupUi(this); + // Single-shot timer used to coalesce the full-forum reloads requested by + // incoming GXS events (see scheduleForumReload()). Created first thing: + // setGroupId(forumId) below reaches updateDisplay(), which touches this timer. + mDeferredReloadTimer = new QTimer(this); + mDeferredReloadTimer->setSingleShot(true); + connect(mDeferredReloadTimer, &QTimer::timeout, this, [this]() { updateDisplay(true); }); + //setUpdateWhenInvisible(true); //mUpdating = false; @@ -375,12 +382,6 @@ GxsForumThreadWidget::GxsForumThreadWidget(const RsGxsGroupId &forumId, QWidget ui->threadTreeWidget->enableColumnCustomize(true); #endif - // Single-shot timer used to coalesce the full-forum reloads requested by - // incoming GXS events (see scheduleForumReload()). - mDeferredReloadTimer = new QTimer(this); - mDeferredReloadTimer->setSingleShot(true); - connect(mDeferredReloadTimer, &QTimer::timeout, this, [this]() { updateDisplay(true); }); - mEventHandlerId = 0; // Needs to be asynced because this function is called by another thread! rsEvents->registerEventsHandler( From 243c417bfcd57b26510ab14879930d9b2ca29e2e Mon Sep 17 00:00:00 2001 From: jolavillette Date: Wed, 29 Jul 2026 12:44:24 +0200 Subject: [PATCH 08/13] GxsForumModel: size the read-status view refresh on rows, not message ids 280e51f53 restricted the dataChanged() refresh to the affected post unless more than 5 messages changed, in which case it refreshes the whole view. But changed_msgs counts message ids, and a post carries one id per stored version of itself, all sharing one read status: marking a single post read or unread on a post edited 46 times queued 47 ids and triggered the whole-model refresh. Measured on a 8259 post forum: one click, one row repainted, 1072 ms of frozen GUI in the dataChanged() handling; with this fix the same click costs about 1 ms. Count the changed rows separately and size the refresh on that. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/gui/gxsforums/GxsForumModel.cpp | 20 +++++++++++++++---- .../src/gui/gxsforums/GxsForumModel.h | 2 +- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp b/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp index 966d3094f..02a28e4de 100644 --- a/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp +++ b/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp @@ -874,7 +874,8 @@ void RsGxsForumModel::setMsgReadStatus(const QModelIndex& i,bool read_status,boo // dataChanged() per post, which froze the UI for a very long time (and could // crash) when marking a large forum (thousands of posts) as read. std::vector changed_msgs; - recursSetMsgReadStatus(entry,read_status,with_children,changed_msgs) ; + uint32_t changed_entries = 0; + recursSetMsgReadStatus(entry,read_status,with_children,changed_msgs,changed_entries) ; bool has_unread_below, has_read_below; recursUpdateReadStatusAndTimes(0,has_unread_below,has_read_below); @@ -898,7 +899,14 @@ void RsGxsForumModel::setMsgReadStatus(const QModelIndex& i,bool read_status,boo rsGxsForums->markRead(grpId, changed_msgs, read_status); }); - if (with_children || changed_msgs.size() > 5) + // How the view is refreshed depends on how many *rows* changed, not on how + // many messages were written. A post keeps every edited version of itself in + // the database and they all share one read status, so marking a single post + // read can queue dozens of message ids while still repainting exactly one + // row. Testing changed_msgs.size() here meant that one click on a post with + // 47 stored versions emitted dataChanged() over the whole model: 1072 ms of + // frozen interface on a 8259 post forum, measured. + if (with_children || changed_entries > 5) { // A single refresh of the whole view instead of one dataChanged() per post. if(mTreeMode == TREE_MODE_FLAT) @@ -917,7 +925,7 @@ void RsGxsForumModel::setMsgReadStatus(const QModelIndex& i,bool read_status,boo } } -void RsGxsForumModel::recursSetMsgReadStatus(ForumModelIndex i,bool read_status,bool with_children,std::vector& changed_msgs) +void RsGxsForumModel::recursSetMsgReadStatus(ForumModelIndex i,bool read_status,bool with_children,std::vector& changed_msgs,uint32_t& changed_entries) { uint32_t newStatus = (read_status ? mPosts[i].mMsgStatus & ~static_cast(GXS_SERV::GXS_MSG_STATUS_GUI_UNREAD) : mPosts[i].mMsgStatus | static_cast(GXS_SERV::GXS_MSG_STATUS_GUI_UNREAD)); @@ -928,6 +936,10 @@ void RsGxsForumModel::recursSetMsgReadStatus(ForumModelIndex i,bool read_status, if (bChanged) { + // One row changed, whatever the number of stored versions collected just + // below. setMsgReadStatus() sizes its view refresh on this count. + ++changed_entries; + //Don't recurs post versions as this should be done before, if no change. auto s = getPostVersions(mPosts[i].mMsgId) ; @@ -945,7 +957,7 @@ void RsGxsForumModel::recursSetMsgReadStatus(ForumModelIndex i,bool read_status, return; for(uint32_t j=0;j& messages); void recursUpdateReadStatusAndTimes(ForumModelIndex i,bool& has_unread_below,bool& has_read_below); uint32_t recursUpdateFilterStatus(ForumModelIndex i,int column,const QStringList& strings); - void recursSetMsgReadStatus(ForumModelIndex i,bool read_status,bool with_children,std::vector& changed_msgs); + void recursSetMsgReadStatus(ForumModelIndex i,bool read_status,bool with_children,std::vector& changed_msgs,uint32_t& changed_entries); static void generateMissingItem(const RsGxsMessageId &msgId,ForumModelPostEntry& entry); static ForumModelIndex addEntry(std::vector& posts,const ForumModelPostEntry& entry,ForumModelIndex parent); From b0ca91c82568d570b868f9722390e3634c3abcf0 Mon Sep 17 00:00:00 2001 From: jolavillette Date: Wed, 29 Jul 2026 12:44:54 +0200 Subject: [PATCH 09/13] gui(forums): drop the expanded-items save/restore around read-status changes setMsgReadStatus() only emits dataChanged(); it never resets the model nor changes its layout, so the expanded items and the current index survive it untouched. Saving and restoring them on every toggle was pure overhead, and an expensive one: the restore walks every expanded item and pays, for each, a linear scan of the post array plus a linear scan of the view items -- quadratic work on the GUI thread for a single post marked read or unread on a large forum. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/gui/gxsforums/GxsForumThreadWidget.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp index 2e75199cf..e37f783cb 100644 --- a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp +++ b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp @@ -1485,7 +1485,6 @@ void GxsForumThreadWidget::markMsgAsReadUnread (bool read, bool children, bool f if (groupId().isNull() || !IS_GROUP_SUBSCRIBED(mForumGroup.mMeta.mSubscribeFlags)) { return; } - saveExpandedItems(mSavedExpandedMessages); QModelIndex src_index; if(forum) @@ -1497,11 +1496,15 @@ void GxsForumThreadWidget::markMsgAsReadUnread (bool read, bool children, bool f else src_index = mThreadModel->getIndexOfMessage(mThreadId); } - mThreadModel->setMsgReadStatus(src_index,read,children); - //Restore Selection - whileBlocking(ui->threadTreeWidget)->setCurrentIndex(mThreadProxyModel->mapFromSource(mThreadModel->getIndexOfMessage(mThreadId))); - recursRestoreExpandedItems(QModelIndex(),mSavedExpandedMessages); + // setMsgReadStatus() only emits dataChanged(): it never resets the model nor + // changes its layout, so neither the expanded items nor the current index are + // lost here. Saving and restoring them was pure overhead -- and not a cheap + // one: restoring walks every expanded item, and each of them costs a linear + // scan of the post array (getIndexOfMessage) plus a linear scan of the view + // items (QTreeView::setExpanded). On a forum with thousands of posts that is + // quadratic work on the GUI thread for every single post marked read. + mThreadModel->setMsgReadStatus(src_index,read,children); } void GxsForumThreadWidget::markMsgAsRead() From 5cbcd962dd1432100596ba083679ff80766211b1 Mon Sep 17 00:00:00 2001 From: jolavillette Date: Wed, 29 Jul 2026 12:45:23 +0200 Subject: [PATCH 10/13] gui(forums): re-read the group on SUBSCRIBE_STATUS_CHANGED instead of flipping the flag The event means the subscription status changed, not that it was toggled: guessing the new value by inverting the current flag goes wrong as soon as one event is duplicated, lost, or refers to a state the widget already holds. When the guess lands on 'unsubscribed', the widget silently refuses to mark posts read or unread (the very first check of markMsgAsReadUnread) until the next group reload puts the flag right again. updateGroupData() was already called anyway; let it be the only writer of the flag. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/gui/gxsforums/GxsForumThreadWidget.cpp | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp index e37f783cb..e71a400b2 100644 --- a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp +++ b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp @@ -415,16 +415,12 @@ void GxsForumThreadWidget::handleEvent_main_thread(std::shared_ptrmForumGroupId == mForumGroup.mMeta.mGroupId) - { - // Toggle subscribe flag locally and refresh UI without GXS request - // to avoid concurrent request with parent dialog's tree rebuild - if(IS_GROUP_SUBSCRIBED(mForumGroup.mMeta.mSubscribeFlags)) - mForumGroup.mMeta.mSubscribeFlags &= ~GXS_SERV::GROUP_SUBSCRIBE_SUBSCRIBED; - else - mForumGroup.mMeta.mSubscribeFlags |= GXS_SERV::GROUP_SUBSCRIBE_SUBSCRIBED; - + // Re-read the group instead of guessing the new flag by flipping + // the current one: the event says the status changed, not that it + // was toggled, and a wrong guess makes the widget believe the + // forum is unsubscribed, which silently disables marking posts + // read or unread until the group data comes back. updateGroupData(); - } break; default: break; From dea2f28dd90b7f6357299d399507e92c843a0d3a Mon Sep 17 00:00:00 2001 From: jolavillette Date: Wed, 29 Jul 2026 12:47:17 +0200 Subject: [PATCH 11/13] gui(forums): opt-in latency probes on the read/unread and loading path Set RS_GUI_PROFILE to a threshold in milliseconds (0 reports everything) to get one line per measured operation on stderr, plus a 50 ms watchdog on the GUI thread that reports every stall of the event loop wherever the blocking code lives, naming the last probed operation. This is what located the actual freeze of the read/unread path (a whole-model dataChanged() measured at 1072 ms) after several plausible-from-code-reading fixes had failed to: the numbers say where the time goes instead of a story explaining where it might go. All probes are inert unless the environment variable is set. Co-Authored-By: Claude Opus 5 (1M context) --- retroshare-gui/src/gui/gxs/GxsPerfProbe.h | 140 ++++++++++++++++++ .../src/gui/gxsforums/GxsForumModel.cpp | 23 +++ .../gui/gxsforums/GxsForumThreadWidget.cpp | 27 +++- .../src/gui/gxsforums/GxsForumThreadWidget.h | 2 +- 4 files changed, 189 insertions(+), 3 deletions(-) create mode 100644 retroshare-gui/src/gui/gxs/GxsPerfProbe.h diff --git a/retroshare-gui/src/gui/gxs/GxsPerfProbe.h b/retroshare-gui/src/gui/gxs/GxsPerfProbe.h new file mode 100644 index 000000000..a41742412 --- /dev/null +++ b/retroshare-gui/src/gui/gxs/GxsPerfProbe.h @@ -0,0 +1,140 @@ +/******************************************************************************* + * retroshare-gui/src/gui/gxs/GxsPerfProbe.h * + * * + * Copyright 2026 by Retroshare Team * + * * + * This program is free software: you can redistribute it and/or modify * + * it under the terms of the GNU Affero General Public License as * + * published by the Free Software Foundation, either version 3 of the * + * License, or (at your option) any later version. * + * * + * This program is distributed in the hope that it will be useful, * + * but WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * + * GNU Affero General Public License for more details. * + * * + * You should have received a copy of the GNU Affero General Public License * + * along with this program. If not, see . * + * * + *******************************************************************************/ + +#pragma once + +// Opt-in latency probes, off unless RS_GUI_PROFILE is set in the environment. +// Its value is a reporting threshold in milliseconds, 0 reports everything: +// +// RS_GUI_PROFILE=0 ./retroshare +// +// Output goes through RsDbg(), i.e. stderr: launch from a terminal or the lines +// go nowhere. + +#include +#include +#include + +#include +#include + +#include "util/rsdebug.h" + +namespace RsGuiPerf { + +inline double threshold() +{ + // <0 means disabled. Read once, the environment does not change at runtime. + static const double t = []() -> double { + const char *v = getenv("RS_GUI_PROFILE"); + if(!v) + v = getenv("RS_FORUM_PROFILE"); // legacy name of the first investigation + return v ? atof(v) : -1.0; + }(); + + return t; +} + +inline bool enabled() { return threshold() >= 0; } + +/* Breadcrumb of the last operation a probe measured on the current thread. The + * stall watchdog prints it, so a stall can be attributed even when the probe + * that covered it stayed below the reporting threshold. */ +inline const char *& lastOp() { static thread_local const char *s = "none"; return s; } +inline double& lastOpMs() { static thread_local double d = 0; return d; } + +/*! + * \brief RAII timer around one operation. + * + * Report a probe placed on the GUI thread as time the interface stayed frozen. + */ +class Probe +{ +public: + explicit Probe(const char *what) + : mWhat(what), mStart(std::chrono::steady_clock::now()) {} + + ~Probe() + { + if(!enabled()) + return; + + const double ms = std::chrono::duration( + std::chrono::steady_clock::now() - mStart ).count(); + + lastOp() = mWhat; + lastOpMs() = ms; + + if(ms >= threshold()) + RsDbg() << "GUI-PROF " << mWhat << " " << mDetails.toStdString() + << " in " << ms << "ms"; + } + + void detail(const QString& s) { mDetails = s; } + +private: + const char *mWhat; + QString mDetails; + std::chrono::steady_clock::time_point mStart; +}; + +/*! + * \brief Watchdog for the GUI thread itself. + * + * The probes above only measure the code they wrap, so they cannot see a stall + * that happens anywhere else. This timer runs on the GUI thread and reports + * whenever the event loop failed to come back on time, whatever the reason and + * wherever the blocking code lives. It also names the last operation a probe + * measured, which points at the culprit when one covers it. + * + * Safe to call several times, only the first call installs anything. + */ +inline void installGuiStallWatchdog() +{ + static bool installed = false; + + if(installed || !enabled()) + return; + + installed = true; + + static const int TICK_MS = 50; + + QTimer *timer = new QTimer(QCoreApplication::instance()); + auto *last = new std::chrono::steady_clock::time_point( + std::chrono::steady_clock::now() ); + + QObject::connect(timer, &QTimer::timeout, QCoreApplication::instance(), [last]() + { + const auto now = std::chrono::steady_clock::now(); + const double ms = std::chrono::duration(now - *last).count(); + *last = now; + + // Anything above the tick plus a comfortable margin means the event loop + // was busy or blocked for that long. + if(ms > TICK_MS + 150) + RsDbg() << "GUI-PROF GUI-THREAD-STALL " << (ms - TICK_MS) + << "ms last_probe=" << lastOp() << " (" << lastOpMs() << "ms)"; + }); + + timer->start(TICK_MS); +} + +} // namespace RsGuiPerf diff --git a/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp b/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp index 02a28e4de..f3d13bd8f 100644 --- a/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp +++ b/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp @@ -30,6 +30,7 @@ #include "gui/gxs/GxsIdDetails.h" #include "gui/gxs/GxsIdTreeWidgetItem.h" #include "GxsForumModel.h" +#include "gui/gxs/GxsPerfProbe.h" #include "retroshare/rsgxsflags.h" #include "retroshare/rsgxsforums.h" #include "retroshare/rsexpr.h" @@ -868,6 +869,16 @@ void RsGxsForumModel::setMsgReadStatus(const QModelIndex& i,bool read_status,boo if(!convertRefPointerToTabEntry(ref,entry) || entry >= mPosts.size()) return ; + RsGuiPerf::Probe prof("model::setMsgReadStatus"); + + auto stamp = std::chrono::steady_clock::now(); + auto lap = [&stamp]() { + const auto now = std::chrono::steady_clock::now(); + const double ms = std::chrono::duration(now-stamp).count(); + stamp = now; + return ms; + }; + // Collect the posts whose read status actually changes and update the // in-memory model right away, but do NOT touch the backend or the view once // per message: doing so used to spawn one detached thread AND emit one @@ -877,9 +888,13 @@ void RsGxsForumModel::setMsgReadStatus(const QModelIndex& i,bool read_status,boo uint32_t changed_entries = 0; recursSetMsgReadStatus(entry,read_status,with_children,changed_msgs,changed_entries) ; + const double ms_collect = lap(); + bool has_unread_below, has_read_below; recursUpdateReadStatusAndTimes(0,has_unread_below,has_read_below); + const double ms_flags = lap(); + // Persist the change(s) in the background so the GUI thread never blocks. // A single interactive read (the common case: selecting/opening one post) // goes through the per-message markRead(), which emits READ_STATUS_CHANGED @@ -923,6 +938,14 @@ void RsGxsForumModel::setMsgReadStatus(const QModelIndex& i,bool read_status,boo emit dataChanged(j, j.sibling(j.row(), COLUMN_THREAD_NB_COLUMNS - 1)); } } + + const double ms_spawn_and_notify = lap(); + + // dataChanged() is emitted synchronously, so the view's reaction to it -- and + // anything the delegates do while repainting -- is accounted for here. + prof.detail(QString("posts=%1 rows=%2 msgs=%3 collect=%4ms flags=%5ms notify=%6ms") + .arg(mPosts.size()).arg(changed_entries).arg(changed_msgs.size()) + .arg(ms_collect).arg(ms_flags).arg(ms_spawn_and_notify)); } void RsGxsForumModel::recursSetMsgReadStatus(ForumModelIndex i,bool read_status,bool with_children,std::vector& changed_msgs,uint32_t& changed_entries) diff --git a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp index e71a400b2..aced2708e 100644 --- a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp +++ b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp @@ -39,6 +39,7 @@ #include "gui/gxs/GxsIdTreeWidgetItem.h" #include "gui/Identity/IdDialog.h" #include "gui/gxs/GxsIdDetails.h" +#include "gui/gxs/GxsPerfProbe.h" #include "util/HandleRichText.h" #include "CreateGxsForumMsg.h" #include "gui/MainWindow.h" @@ -60,6 +61,8 @@ //#define DEBUG_FORUMS +using RsGuiPerf::Probe; + /* Images for context menu icons */ #define IMAGE_MESSAGE ":/icons/mail/compose.png" #define IMAGE_REPLY ":/icons/mail/reply.png" @@ -254,6 +257,8 @@ GxsForumThreadWidget::GxsForumThreadWidget(const RsGxsGroupId &forumId, QWidget { ui->setupUi(this); + RsGuiPerf::installGuiStallWatchdog(); + // Single-shot timer used to coalesce the full-forum reloads requested by // incoming GXS events (see scheduleForumReload()). Created first thing: // setGroupId(forumId) below reaches updateDisplay(), which touches this timer. @@ -410,7 +415,7 @@ void GxsForumThreadWidget::handleEvent_main_thread(std::shared_ptrmForumGroupId == mForumGroup.mMeta.mGroupId) - scheduleForumReload(); + scheduleForumReload(static_cast(e->mForumEventCode)); break; case RsForumEventCode::SUBSCRIBE_STATUS_CHANGED: @@ -428,8 +433,14 @@ void GxsForumThreadWidget::handleEvent_main_thread(std::shared_ptrroot(); @@ -1924,6 +1944,9 @@ void GxsForumThreadWidget::filterItems(const QString& text) void GxsForumThreadWidget::postForumLoading() { + Probe prof("postForumLoading"); + prof.detail(QString("expanded_items=%1").arg(mSavedExpandedMessages.size())); + if(groupId().isNull()) { ui->nextUnreadButton->setEnabled(false); diff --git a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.h b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.h index 130f70bd0..ba3bd52ab 100644 --- a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.h +++ b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.h @@ -203,7 +203,7 @@ private: // forum for each one froze the UI for seconds and reset the model out from // under the user's selection. Restart a single-shot timer instead so a burst // results in one reload once the events settle. - void scheduleForumReload(); + void scheduleForumReload(int event_code); private: void setForumDescriptionLoading(); From ebac1481f0cbee4eb3ce484c95b5a40d0e8b6c05 Mon Sep 17 00:00:00 2001 From: jolavillette Date: Sat, 1 Aug 2026 23:03:27 +0200 Subject: [PATCH 12/13] Revert "gui(forums): opt-in latency probes on the read/unread and loading path" Profiling code removed from the PR, as requested in review of the sibling PRs. This reverts commit 8845aa4636e630977f4a6817e8a267735e128e28. Co-Authored-By: Claude Fable 5 --- retroshare-gui/src/gui/gxs/GxsPerfProbe.h | 140 ------------------ .../src/gui/gxsforums/GxsForumModel.cpp | 23 --- .../gui/gxsforums/GxsForumThreadWidget.cpp | 27 +--- .../src/gui/gxsforums/GxsForumThreadWidget.h | 2 +- 4 files changed, 3 insertions(+), 189 deletions(-) delete mode 100644 retroshare-gui/src/gui/gxs/GxsPerfProbe.h diff --git a/retroshare-gui/src/gui/gxs/GxsPerfProbe.h b/retroshare-gui/src/gui/gxs/GxsPerfProbe.h deleted file mode 100644 index a41742412..000000000 --- a/retroshare-gui/src/gui/gxs/GxsPerfProbe.h +++ /dev/null @@ -1,140 +0,0 @@ -/******************************************************************************* - * retroshare-gui/src/gui/gxs/GxsPerfProbe.h * - * * - * Copyright 2026 by Retroshare Team * - * * - * This program is free software: you can redistribute it and/or modify * - * it under the terms of the GNU Affero General Public License as * - * published by the Free Software Foundation, either version 3 of the * - * License, or (at your option) any later version. * - * * - * This program is distributed in the hope that it will be useful, * - * but WITHOUT ANY WARRANTY; without even the implied warranty of * - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * - * GNU Affero General Public License for more details. * - * * - * You should have received a copy of the GNU Affero General Public License * - * along with this program. If not, see . * - * * - *******************************************************************************/ - -#pragma once - -// Opt-in latency probes, off unless RS_GUI_PROFILE is set in the environment. -// Its value is a reporting threshold in milliseconds, 0 reports everything: -// -// RS_GUI_PROFILE=0 ./retroshare -// -// Output goes through RsDbg(), i.e. stderr: launch from a terminal or the lines -// go nowhere. - -#include -#include -#include - -#include -#include - -#include "util/rsdebug.h" - -namespace RsGuiPerf { - -inline double threshold() -{ - // <0 means disabled. Read once, the environment does not change at runtime. - static const double t = []() -> double { - const char *v = getenv("RS_GUI_PROFILE"); - if(!v) - v = getenv("RS_FORUM_PROFILE"); // legacy name of the first investigation - return v ? atof(v) : -1.0; - }(); - - return t; -} - -inline bool enabled() { return threshold() >= 0; } - -/* Breadcrumb of the last operation a probe measured on the current thread. The - * stall watchdog prints it, so a stall can be attributed even when the probe - * that covered it stayed below the reporting threshold. */ -inline const char *& lastOp() { static thread_local const char *s = "none"; return s; } -inline double& lastOpMs() { static thread_local double d = 0; return d; } - -/*! - * \brief RAII timer around one operation. - * - * Report a probe placed on the GUI thread as time the interface stayed frozen. - */ -class Probe -{ -public: - explicit Probe(const char *what) - : mWhat(what), mStart(std::chrono::steady_clock::now()) {} - - ~Probe() - { - if(!enabled()) - return; - - const double ms = std::chrono::duration( - std::chrono::steady_clock::now() - mStart ).count(); - - lastOp() = mWhat; - lastOpMs() = ms; - - if(ms >= threshold()) - RsDbg() << "GUI-PROF " << mWhat << " " << mDetails.toStdString() - << " in " << ms << "ms"; - } - - void detail(const QString& s) { mDetails = s; } - -private: - const char *mWhat; - QString mDetails; - std::chrono::steady_clock::time_point mStart; -}; - -/*! - * \brief Watchdog for the GUI thread itself. - * - * The probes above only measure the code they wrap, so they cannot see a stall - * that happens anywhere else. This timer runs on the GUI thread and reports - * whenever the event loop failed to come back on time, whatever the reason and - * wherever the blocking code lives. It also names the last operation a probe - * measured, which points at the culprit when one covers it. - * - * Safe to call several times, only the first call installs anything. - */ -inline void installGuiStallWatchdog() -{ - static bool installed = false; - - if(installed || !enabled()) - return; - - installed = true; - - static const int TICK_MS = 50; - - QTimer *timer = new QTimer(QCoreApplication::instance()); - auto *last = new std::chrono::steady_clock::time_point( - std::chrono::steady_clock::now() ); - - QObject::connect(timer, &QTimer::timeout, QCoreApplication::instance(), [last]() - { - const auto now = std::chrono::steady_clock::now(); - const double ms = std::chrono::duration(now - *last).count(); - *last = now; - - // Anything above the tick plus a comfortable margin means the event loop - // was busy or blocked for that long. - if(ms > TICK_MS + 150) - RsDbg() << "GUI-PROF GUI-THREAD-STALL " << (ms - TICK_MS) - << "ms last_probe=" << lastOp() << " (" << lastOpMs() << "ms)"; - }); - - timer->start(TICK_MS); -} - -} // namespace RsGuiPerf diff --git a/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp b/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp index f3d13bd8f..02a28e4de 100644 --- a/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp +++ b/retroshare-gui/src/gui/gxsforums/GxsForumModel.cpp @@ -30,7 +30,6 @@ #include "gui/gxs/GxsIdDetails.h" #include "gui/gxs/GxsIdTreeWidgetItem.h" #include "GxsForumModel.h" -#include "gui/gxs/GxsPerfProbe.h" #include "retroshare/rsgxsflags.h" #include "retroshare/rsgxsforums.h" #include "retroshare/rsexpr.h" @@ -869,16 +868,6 @@ void RsGxsForumModel::setMsgReadStatus(const QModelIndex& i,bool read_status,boo if(!convertRefPointerToTabEntry(ref,entry) || entry >= mPosts.size()) return ; - RsGuiPerf::Probe prof("model::setMsgReadStatus"); - - auto stamp = std::chrono::steady_clock::now(); - auto lap = [&stamp]() { - const auto now = std::chrono::steady_clock::now(); - const double ms = std::chrono::duration(now-stamp).count(); - stamp = now; - return ms; - }; - // Collect the posts whose read status actually changes and update the // in-memory model right away, but do NOT touch the backend or the view once // per message: doing so used to spawn one detached thread AND emit one @@ -888,13 +877,9 @@ void RsGxsForumModel::setMsgReadStatus(const QModelIndex& i,bool read_status,boo uint32_t changed_entries = 0; recursSetMsgReadStatus(entry,read_status,with_children,changed_msgs,changed_entries) ; - const double ms_collect = lap(); - bool has_unread_below, has_read_below; recursUpdateReadStatusAndTimes(0,has_unread_below,has_read_below); - const double ms_flags = lap(); - // Persist the change(s) in the background so the GUI thread never blocks. // A single interactive read (the common case: selecting/opening one post) // goes through the per-message markRead(), which emits READ_STATUS_CHANGED @@ -938,14 +923,6 @@ void RsGxsForumModel::setMsgReadStatus(const QModelIndex& i,bool read_status,boo emit dataChanged(j, j.sibling(j.row(), COLUMN_THREAD_NB_COLUMNS - 1)); } } - - const double ms_spawn_and_notify = lap(); - - // dataChanged() is emitted synchronously, so the view's reaction to it -- and - // anything the delegates do while repainting -- is accounted for here. - prof.detail(QString("posts=%1 rows=%2 msgs=%3 collect=%4ms flags=%5ms notify=%6ms") - .arg(mPosts.size()).arg(changed_entries).arg(changed_msgs.size()) - .arg(ms_collect).arg(ms_flags).arg(ms_spawn_and_notify)); } void RsGxsForumModel::recursSetMsgReadStatus(ForumModelIndex i,bool read_status,bool with_children,std::vector& changed_msgs,uint32_t& changed_entries) diff --git a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp index aced2708e..e71a400b2 100644 --- a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp +++ b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp @@ -39,7 +39,6 @@ #include "gui/gxs/GxsIdTreeWidgetItem.h" #include "gui/Identity/IdDialog.h" #include "gui/gxs/GxsIdDetails.h" -#include "gui/gxs/GxsPerfProbe.h" #include "util/HandleRichText.h" #include "CreateGxsForumMsg.h" #include "gui/MainWindow.h" @@ -61,8 +60,6 @@ //#define DEBUG_FORUMS -using RsGuiPerf::Probe; - /* Images for context menu icons */ #define IMAGE_MESSAGE ":/icons/mail/compose.png" #define IMAGE_REPLY ":/icons/mail/reply.png" @@ -257,8 +254,6 @@ GxsForumThreadWidget::GxsForumThreadWidget(const RsGxsGroupId &forumId, QWidget { ui->setupUi(this); - RsGuiPerf::installGuiStallWatchdog(); - // Single-shot timer used to coalesce the full-forum reloads requested by // incoming GXS events (see scheduleForumReload()). Created first thing: // setGroupId(forumId) below reaches updateDisplay(), which touches this timer. @@ -415,7 +410,7 @@ void GxsForumThreadWidget::handleEvent_main_thread(std::shared_ptrmForumGroupId == mForumGroup.mMeta.mGroupId) - scheduleForumReload(static_cast(e->mForumEventCode)); + scheduleForumReload(); break; case RsForumEventCode::SUBSCRIBE_STATUS_CHANGED: @@ -433,14 +428,8 @@ void GxsForumThreadWidget::handleEvent_main_thread(std::shared_ptrroot(); @@ -1944,9 +1924,6 @@ void GxsForumThreadWidget::filterItems(const QString& text) void GxsForumThreadWidget::postForumLoading() { - Probe prof("postForumLoading"); - prof.detail(QString("expanded_items=%1").arg(mSavedExpandedMessages.size())); - if(groupId().isNull()) { ui->nextUnreadButton->setEnabled(false); diff --git a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.h b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.h index ba3bd52ab..130f70bd0 100644 --- a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.h +++ b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.h @@ -203,7 +203,7 @@ private: // forum for each one froze the UI for seconds and reset the model out from // under the user's selection. Restart a single-shot timer instead so a burst // results in one reload once the events settle. - void scheduleForumReload(int event_code); + void scheduleForumReload(); private: void setForumDescriptionLoading(); From 1b99beebc5c4811d7ca16b53539cfc51f42e05ae Mon Sep 17 00:00:00 2001 From: jolavillette Date: Sat, 1 Aug 2026 23:03:27 +0200 Subject: [PATCH 13/13] Address review: pointer hand-off to the workers, event-driven board update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - GxsForumModel / PostedPostsModel: hand the id list to the background worker through a pointer deleted after the call, so the lambda capture does not deep copy it. - PostedPostsModel: setAllMsgReadStatus() no longer updates the model directly. The batched READ_STATUS_CHANGED event now carries the affected ids and their new state (libretroshare side), and the event handler applies them locally — so a batch initiated by another frontend (e.g. webUI) updates the board view exactly the same way, and no post is re-read from the database. Co-Authored-By: Claude Fable 5 --- .../src/gui/Posted/PostedPostsModel.cpp | 49 ++++++++++++++----- .../src/gui/gxsforums/GxsForumModel.cpp | 11 ++++- 2 files changed, 46 insertions(+), 14 deletions(-) diff --git a/retroshare-gui/src/gui/Posted/PostedPostsModel.cpp b/retroshare-gui/src/gui/Posted/PostedPostsModel.cpp index cd014168a..e827a2fbb 100644 --- a/retroshare-gui/src/gui/Posted/PostedPostsModel.cpp +++ b/retroshare-gui/src/gui/Posted/PostedPostsModel.cpp @@ -79,6 +79,31 @@ void RsPostedPostsModel::handleEvent_main_thread(std::shared_ptr case RsPostedEventCode::MESSAGE_VOTES_UPDATED: case RsPostedEventCode::NEW_MESSAGE: { + // Batched read-status change ("mark all as read", possibly issued by + // another frontend such as the webUI): the event carries the affected + // message ids and their new state, so update the flags locally + // instead of re-reading the posts from the database. + if( e->mPostedEventCode == RsPostedEventCode::READ_STATUS_CHANGED + && !e->mPostedMsgIds.empty() ) + { + if(e->mPostedGroupId != mPostedGroup.mMeta.mGroupId) + return; + + const std::set ids(e->mPostedMsgIds.begin(),e->mPostedMsgIds.end()); + + for(uint32_t i=0;imPostedMsgsRead) + mPosts[i].mMeta.mMsgStatus &= ~(GXS_SERV::GXS_MSG_STATUS_GUI_UNREAD | GXS_SERV::GXS_MSG_STATUS_GUI_NEW); + else + mPosts[i].mMeta.mMsgStatus |= GXS_SERV::GXS_MSG_STATUS_GUI_UNREAD; + } + + emit dataChanged(createIndex(0,0,(void*)NULL), createIndex(mDisplayedNbPosts-1,0,(void*)NULL)); + return; + } + // Normally we should just emit dataChanged() on the index of the data that has changed: // // We need to update the data! @@ -754,21 +779,21 @@ void RsPostedPostsModel::setAllMsgReadStatus(bool read) } if(!msgIds.empty()) - RsThread::async([boardId=mPostedGroup.mMeta.mGroupId, msgIds, read]() + { + // Hand the id list over through a pointer: the lambda capture would + // otherwise deep copy it. + auto* ids = new std::vector(std::move(msgIds)); + + RsThread::async([boardId=mPostedGroup.mMeta.mGroupId, ids, read]() { - rsPosted->setPostReadStatus(boardId, msgIds, read); + rsPosted->setPostReadStatus(boardId, *ids, read); + delete ids; } ); + } - // Update the local model immediately, since we don't catch the resulting - // event later (that would reload the posts). - - for(uint32_t i=0;imarkRead(std::make_pair(grpId, msgId), read_status); }); else if(!changed_msgs.empty()) - RsThread::async( [grpId=mForumGroup.mMeta.mGroupId,changed_msgs,read_status]() + { + // Hand the (possibly long) id list over through a pointer: the lambda + // capture would otherwise deep copy it. + auto* msgs = new std::vector(std::move(changed_msgs)); + + RsThread::async( [grpId=mForumGroup.mMeta.mGroupId,msgs,read_status]() { - rsGxsForums->markRead(grpId, changed_msgs, read_status); + rsGxsForums->markRead(grpId, *msgs, read_status); + delete msgs; }); + } // How the view is refreshed depends on how many *rows* changed, not on how // many messages were written. A post keeps every edited version of itself in