From 95ca2cbc52d97a858493cd5c085cc7caeb44d115 Mon Sep 17 00:00:00 2001 From: jolavillette Date: Wed, 29 Jul 2026 17:04:39 +0200 Subject: [PATCH] Revert the statistics throttling: it slowed the counters down Two earlier commits of this branch throttled how group statistics are requested: a bounded window of 8 concurrent jobs, and a 700 ms debounce on read-status events. Both are reverted. They were aimed at the wrong end of the chain. The bottleneck is not concurrency, it is one query: reading a forum's statistics visits every message row of that group, and on a large forum with a cold cache that single request holds the single-threaded GXS engine for tens of seconds (26 s measured on a 9573-message forum). Throttling the submission side only delays the cheap requests -- the small forums, whose counters used to appear within seconds, now queue behind a request that blocks the engine anyway. Measured effect on a real profile: counters took tens of seconds where they previously took a few. What the branch keeps is what actually removes work: not recomputing local statistics on a network STATISTICS_CHANGED event, and re-applying the cached local counts after a group tree refill. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/gui/gxs/GxsGroupFrameDialog.cpp | 46 ++++--------------- .../src/gui/gxs/GxsGroupFrameDialog.h | 28 ----------- .../src/gui/gxsforums/GxsForumsDialog.cpp | 21 +-------- .../src/gui/gxsforums/GxsForumsDialog.h | 10 ---- 4 files changed, 11 insertions(+), 94 deletions(-) diff --git a/retroshare-gui/src/gui/gxs/GxsGroupFrameDialog.cpp b/retroshare-gui/src/gui/gxs/GxsGroupFrameDialog.cpp index fbb675f6f..ef897637f 100644 --- a/retroshare-gui/src/gui/gxs/GxsGroupFrameDialog.cpp +++ b/retroshare-gui/src/gui/gxs/GxsGroupFrameDialog.cpp @@ -82,7 +82,6 @@ GxsGroupFrameDialog::GxsGroupFrameDialog(RsGxsIfaceHelper *ifaceImpl,const QStri mShouldUpdateMessageSummaryList = true; mShouldUpdateGroupStatistics = false; mLastGroupStatisticsUpdateTs=0; - mStatisticsJobsInFlight = 0; mInitialized = false; mDistSyncAllowed = allow_dist_sync; mInFill = false; @@ -1308,27 +1307,6 @@ void GxsGroupFrameDialog::updateGroupStatistics(const RsGxsGroupId &groupId) } void GxsGroupFrameDialog::updateGroupStatisticsReal(const RsGxsGroupId &groupId) -{ - // Queue rather than start: see startStatisticsJobs() for why the number of - // concurrent requests has to stay bounded. - mStatisticsQueue.insert(groupId); - - startStatisticsJobs(); -} - -void GxsGroupFrameDialog::startStatisticsJobs() -{ - while(mStatisticsJobsInFlight < MAX_CONCURRENT_STATISTICS_JOBS && !mStatisticsQueue.empty()) - { - const RsGxsGroupId groupId = *mStatisticsQueue.begin(); - mStatisticsQueue.erase(mStatisticsQueue.begin()); - - ++mStatisticsJobsInFlight; - startOneStatisticsJob(groupId); - } -} - -void GxsGroupFrameDialog::startOneStatisticsJob(const RsGxsGroupId &groupId) { RsThread::async([this,groupId]() { @@ -1345,9 +1323,12 @@ void GxsGroupFrameDialog::startOneStatisticsJob(const RsGxsGroupId &groupId) } if(!ok) + { std::cerr << __PRETTY_FUNCTION__ << " failed to collect group statistics for group " << groupId << std::endl; + return; + } - RsQThreadUtils::postToObject( [this,stats,groupId,ok]() + RsQThreadUtils::postToObject( [this,stats,groupId]() { RsGuiPerf::Probe prof("groupStatistics(UI apply)"); @@ -1357,22 +1338,15 @@ void GxsGroupFrameDialog::startOneStatisticsJob(const RsGxsGroupId &groupId) * Qt::QueuedConnection is important! */ - if(ok) - { - QTreeWidgetItem *item = ui->groupTreeWidget->getItemFromId(QString::fromStdString(stats.mGrpId.toStdString())); + QTreeWidgetItem *item = ui->groupTreeWidget->getItemFromId(QString::fromStdString(stats.mGrpId.toStdString())); - if (item) - // ui->groupTreeWidget->setUnreadCount(item, mCountChildMsgs ? (stats.mNumThreadMsgsUnread + stats.mNumChildMsgsUnread) : stats.mNumThreadMsgsUnread); - ui->groupTreeWidget->setCounts(item, mCountChildMsgs ? (stats.mNumThreadMsgsUnread + stats.mNumChildMsgsUnread) : stats.mNumThreadMsgsUnread, stats.mNumMsgs); + if (item) + // ui->groupTreeWidget->setUnreadCount(item, mCountChildMsgs ? (stats.mNumThreadMsgsUnread + stats.mNumChildMsgsUnread) : stats.mNumThreadMsgsUnread); + ui->groupTreeWidget->setCounts(item, mCountChildMsgs ? (stats.mNumThreadMsgsUnread + stats.mNumChildMsgsUnread) : stats.mNumThreadMsgsUnread, stats.mNumMsgs); - mCachedGroupStats[groupId] = stats; + mCachedGroupStats[groupId] = stats; - getUserNotify()->updateIcon(); - } - - // Free the slot and refill the window. - --mStatisticsJobsInFlight; - startStatisticsJobs(); + getUserNotify()->updateIcon(); }, this ); }); diff --git a/retroshare-gui/src/gui/gxs/GxsGroupFrameDialog.h b/retroshare-gui/src/gui/gxs/GxsGroupFrameDialog.h index d4d414a8c..9a198b6d1 100644 --- a/retroshare-gui/src/gui/gxs/GxsGroupFrameDialog.h +++ b/retroshare-gui/src/gui/gxs/GxsGroupFrameDialog.h @@ -195,37 +195,9 @@ protected: virtual void updateGroupStatisticsReal(const RsGxsGroupId &groupId); private: - /*! - * rief Runs the queued group statistics with a bounded number in flight. - * - * Computing the statistics of one group is a blocking GXS request served by - * a single engine thread that sleeps 100 ms between passes, so most of a - * job's duration is waiting, not work: median 100 ms per group, which is - * exactly the tick quantum. Two extremes are both wrong. - * - * One detached thread per group: 839 jobs completing in the same second, - * and because each gives up after 5 s (waitToken cap) the ones at the back - * time out, are cancelled, and their counter never appears -- 104 of them - * measured. - * - * Strictly one at a time: no failures, but the 100 ms of latency is paid - * once per group in sequence -- 630 groups took 172 s before every counter - * showed up, and one slow group (a cold-cache forum taking 28 s) blocked - * all the others behind it. - * - * A small window keeps several requests waiting on the same tick pass, so - * they are served together, while staying far from the failure threshold. - */ - void startStatisticsJobs(); - void startOneStatisticsJob(const RsGxsGroupId &groupId); - /*! Re-apply the known local post/unread counts after a group tree refill. */ void restoreCachedGroupCounts(); - static const int MAX_CONCURRENT_STATISTICS_JOBS = 8; - - std::set mStatisticsQueue; - int mStatisticsJobsInFlight; protected: diff --git a/retroshare-gui/src/gui/gxsforums/GxsForumsDialog.cpp b/retroshare-gui/src/gui/gxsforums/GxsForumsDialog.cpp index ce4a02229..2e47f3f38 100644 --- a/retroshare-gui/src/gui/gxsforums/GxsForumsDialog.cpp +++ b/retroshare-gui/src/gui/gxsforums/GxsForumsDialog.cpp @@ -51,18 +51,6 @@ GxsForumsDialog::GxsForumsDialog(QWidget *parent) : mUpdateTimer = new QTimer(this); mUpdateTimer->setSingleShot(true); connect(mUpdateTimer, SIGNAL(timeout()), this, SLOT(timerUpdate())); - - mStatisticsDebounceTimer = new QTimer(this); - mStatisticsDebounceTimer->setSingleShot(true); - connect(mStatisticsDebounceTimer, SIGNAL(timeout()), this, SLOT(flushPendingStatistics())); -} - -void GxsForumsDialog::flushPendingStatistics() -{ - for(const RsGxsGroupId& groupId: mStatisticsPending) - updateGroupStatisticsReal(groupId); - - mStatisticsPending.clear(); } void GxsForumsDialog::handleEvent_main_thread(std::shared_ptr event) @@ -81,14 +69,7 @@ void GxsForumsDialog::handleEvent_main_thread(std::shared_ptr eve case RsForumEventCode::NEW_MESSAGE: case RsForumEventCode::UPDATED_MESSAGE: // [[fallthrough]]; case RsForumEventCode::READ_STATUS_CHANGED: - // Debounced rather than immediate: one recomputation costs a full - // rebuild of the forum's post hierarchy, and marking posts read or - // unread produces one event each. 700 ms still reads as instant. - // The timer is not restarted by later events, so a long burst (a - // sync delivering messages) cannot postpone the update forever. - mStatisticsPending.insert(e->mForumGroupId); - if(!mStatisticsDebounceTimer->isActive()) - mStatisticsDebounceTimer->start(700); + updateGroupStatisticsReal(e->mForumGroupId); // update the list immediately break; case RsForumEventCode::NEW_FORUM: // [[fallthrough]]; diff --git a/retroshare-gui/src/gui/gxsforums/GxsForumsDialog.h b/retroshare-gui/src/gui/gxsforums/GxsForumsDialog.h index b5cf2f804..d6451c779 100644 --- a/retroshare-gui/src/gui/gxsforums/GxsForumsDialog.h +++ b/retroshare-gui/src/gui/gxsforums/GxsForumsDialog.h @@ -23,8 +23,6 @@ #include "gui/gxs/GxsGroupFrameDialog.h" -#include - #define IMAGE_GXSFORUMS ":/icons/png/forums.png" class GxsForumsDialog : public GxsGroupFrameDialog @@ -70,16 +68,8 @@ private: QTimer *mUpdateTimer; - // Read/unread events are debounced: recomputing a forum's statistics means - // rebuilding its whole post hierarchy (see p3GxsForums::getForumStatistics), - // so doing it on every single post marked read makes navigation crawl on a - // large forum. Bursts collapse into one recomputation per forum. - QTimer *mStatisticsDebounceTimer; - std::set mStatisticsPending; - private slots: void timerUpdate() { updateDisplay(true); } - void flushPendingStatistics(); }; #endif