From ebac1481f0cbee4eb3ce484c95b5a40d0e8b6c05 Mon Sep 17 00:00:00 2001 From: jolavillette Date: Sat, 1 Aug 2026 23:03:27 +0200 Subject: [PATCH] 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();