From 54d4c791aa1ab50dced77054494c2d7bfb23106e Mon Sep 17 00:00:00 2001 From: jolavillette Date: Mon, 27 Jul 2026 11:03:45 +0200 Subject: [PATCH 1/3] GUI: profile the channel post model loading Complements the GXS side instrumentation (libretroshare, gxs/rsgxsprofiler.h) with the two phases that happen in the GUI: the service calls made from the loader thread in update_posts(), and the model update in setPosts(), which runs in the Qt thread and is what actually freezes the interface. setPosts() reports its three phases separately (array copy, sort, view update) since they have very different causes. Enabled by the same RS_GXS_PROFILE environment variable, off by default. Co-Authored-By: Claude Opus 5 (1M context) --- .../gui/gxschannels/GxsChannelPostsModel.cpp | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/retroshare-gui/src/gui/gxschannels/GxsChannelPostsModel.cpp b/retroshare-gui/src/gui/gxschannels/GxsChannelPostsModel.cpp index 45b465955..ee3c0e689 100644 --- a/retroshare-gui/src/gui/gxschannels/GxsChannelPostsModel.cpp +++ b/retroshare-gui/src/gui/gxschannels/GxsChannelPostsModel.cpp @@ -26,6 +26,7 @@ #include "retroshare/rsgxsflags.h" #include "retroshare/rsgxschannels.h" #include "retroshare/rsexpr.h" +#include "gxs/rsgxsprofiler.h" #include "gui/MainWindow.h" #include "gui/mainpagestack.h" @@ -528,6 +529,8 @@ void RsGxsChannelPostsModel::updateSinglePost(const RsGxsChannelPost& post,std:: void RsGxsChannelPostsModel::setPosts(const RsGxsChannelGroup& group, std::vector& posts) { + RsGxsProfiler::Timer prof_timer; + preMods(); initEmptyHierarchy(); @@ -536,8 +539,13 @@ void RsGxsChannelPostsModel::setPosts(const RsGxsChannelGroup& group, std::vecto // createPostsArray(posts); mPosts = posts; + + const long prof_copy_ms = prof_timer.lap(); + std::sort(mPosts.begin(),mPosts.end()); + const long prof_sort_ms = prof_timer.lap(); + for(uint32_t i=0;i channelIds; @@ -583,6 +602,8 @@ void RsGxsChannelPostsModel::update_posts(const RsGxsGroupId& group_id) RsGxsChannelGroup group = groups[0]; + const long prof_grpinfo_ms = prof_timer.lap(); + std::vector *posts = new std::vector(); // We use the heap because the arrays need to be stored accross async std::vector comments ; std::vector votes ; @@ -592,6 +613,14 @@ void RsGxsChannelPostsModel::update_posts(const RsGxsGroupId& group_id) std::cerr << __PRETTY_FUNCTION__ << " failed to retrieve channel messages for channel " << group_id << std::endl; return; } + + const long prof_content_ms = prof_timer.ms(); + + RS_GXS_PROF( prof_grpinfo_ms + prof_content_ms, + "update_posts grp=" << group_id << " posts=" << posts->size() + << " getChannelsInfo=" << prof_grpinfo_ms << "ms" + << " getChannelAllContent=" << prof_content_ms << "ms" + << " service_total=" << (prof_grpinfo_ms + prof_content_ms) << "ms" ); #ifdef DEBUG_CHANNEL_MODEL std::cerr << "Got channel all content for channel " << group_id << std::endl; std::cerr << " posts : " << posts->size() << std::endl; From 28e7f791ff5a89925e07fc6ba48e99e8e821a5ee Mon Sep 17 00:00:00 2001 From: jolavillette Date: Mon, 27 Jul 2026 11:11:56 +0200 Subject: [PATCH 2/3] GUI: stop copying and sorting the channel post array in the Qt thread setPosts() runs in the Qt thread, through postToObject(). It copied the whole post array a fourth time and then sorted it there, so for a channel with a few thousand posts the interface was frozen for the duration of both. Move the sort into the loader thread in update_posts(), where the array is already sitting after the service call, and have setPosts() take its argument by rvalue reference so the array is moved in rather than copied. The sort is kept in setPosts() as a safety net: on an already ordered array it only runs comparisons and moves nothing. Combined with the libretroshare side making posts movable, the array is now handed from the data store to the model without a single deep copy of a thumbnail. Co-Authored-By: Claude Opus 5 (1M context) --- .../gui/gxschannels/GxsChannelPostsModel.cpp | 29 +++++++++++++++---- .../gui/gxschannels/GxsChannelPostsModel.h | 3 +- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/retroshare-gui/src/gui/gxschannels/GxsChannelPostsModel.cpp b/retroshare-gui/src/gui/gxschannels/GxsChannelPostsModel.cpp index ee3c0e689..7e3017428 100644 --- a/retroshare-gui/src/gui/gxschannels/GxsChannelPostsModel.cpp +++ b/retroshare-gui/src/gui/gxschannels/GxsChannelPostsModel.cpp @@ -527,7 +527,7 @@ void RsGxsChannelPostsModel::updateSinglePost(const RsGxsChannelPost& post,std:: triggerViewUpdate(true,false); } -void RsGxsChannelPostsModel::setPosts(const RsGxsChannelGroup& group, std::vector& posts) +void RsGxsChannelPostsModel::setPosts(const RsGxsChannelGroup& group, std::vector&& posts) { RsGxsProfiler::Timer prof_timer; @@ -538,14 +538,21 @@ void RsGxsChannelPostsModel::setPosts(const RsGxsChannelGroup& group, std::vecto // createPostsArray(posts); - mPosts = posts; + // The caller hands over its array and discards it right after, so take it + // rather than deep copying every post (and every thumbnail) one more time. + mPosts = std::move(posts); const long prof_copy_ms = prof_timer.lap(); + // update_posts() already sorted the array in its loader thread. Kept as a + // cheap safety net for any other caller: on already ordered input this only + // costs comparisons, no element is moved. std::sort(mPosts.begin(),mPosts.end()); const long prof_sort_ms = prof_timer.lap(); + mFilteredPosts.reserve(mPosts.size()); + for(uint32_t i=0;ibegin(),posts->end()); + + const long prof_sort_ms = prof_timer.ms(); + const long prof_total_ms = prof_grpinfo_ms + prof_content_ms + prof_sort_ms; + + RS_GXS_PROF( prof_total_ms, "update_posts grp=" << group_id << " posts=" << posts->size() << " getChannelsInfo=" << prof_grpinfo_ms << "ms" << " getChannelAllContent=" << prof_content_ms << "ms" - << " service_total=" << (prof_grpinfo_ms + prof_content_ms) << "ms" ); + << " sort=" << prof_sort_ms << "ms" + << " service_total=" << prof_total_ms << "ms" ); #ifdef DEBUG_CHANNEL_MODEL std::cerr << "Got channel all content for channel " << group_id << std::endl; std::cerr << " posts : " << posts->size() << std::endl; @@ -638,7 +655,7 @@ void RsGxsChannelPostsModel::update_posts(const RsGxsGroupId& group_id) * Qt::QueuedConnection is important! */ - setPosts(group,*posts) ; + setPosts(group,std::move(*posts)) ; delete posts; diff --git a/retroshare-gui/src/gui/gxschannels/GxsChannelPostsModel.h b/retroshare-gui/src/gui/gxschannels/GxsChannelPostsModel.h index ad69e7594..a13712a41 100644 --- a/retroshare-gui/src/gui/gxschannels/GxsChannelPostsModel.h +++ b/retroshare-gui/src/gui/gxschannels/GxsChannelPostsModel.h @@ -239,7 +239,8 @@ private: //void computeMessagesHierarchy(const RsGxsChannelGroup& forum_group, const std::vector &msgs_array, std::vector &posts, std::map > > &mPostVersions); void old_createPostsArray(std::vector &posts); void createPostsArray(std::vector& posts); - void setPosts(const RsGxsChannelGroup& group, std::vector &posts); + /// Takes ownership of posts: the array is moved into the model. + void setPosts(const RsGxsChannelGroup& group, std::vector &&posts); public: void updateSinglePost(const RsGxsChannelPost& post, std::set& added_files, std::set& removed_files); private: From 0e31d5fed03418507faeee11fc05383c0c0af888 Mon Sep 17 00:00:00 2001 From: jolavillette Date: Sat, 1 Aug 2026 14:31:38 +0200 Subject: [PATCH 3/3] GUI: remove the profiling probes from the channel post model Requested in review: the RS_GXS_PROFILE instrumentation of update_posts() and setPosts() served to measure the freeze and verify the fix, but it should not stay in the optimised code. The libretroshare profiling header is no longer included. Co-Authored-By: Claude Fable 5 --- .../gui/gxschannels/GxsChannelPostsModel.cpp | 31 ------------------- 1 file changed, 31 deletions(-) diff --git a/retroshare-gui/src/gui/gxschannels/GxsChannelPostsModel.cpp b/retroshare-gui/src/gui/gxschannels/GxsChannelPostsModel.cpp index 7e3017428..c5a4b51cc 100644 --- a/retroshare-gui/src/gui/gxschannels/GxsChannelPostsModel.cpp +++ b/retroshare-gui/src/gui/gxschannels/GxsChannelPostsModel.cpp @@ -26,7 +26,6 @@ #include "retroshare/rsgxsflags.h" #include "retroshare/rsgxschannels.h" #include "retroshare/rsexpr.h" -#include "gxs/rsgxsprofiler.h" #include "gui/MainWindow.h" #include "gui/mainpagestack.h" @@ -529,8 +528,6 @@ void RsGxsChannelPostsModel::updateSinglePost(const RsGxsChannelPost& post,std:: void RsGxsChannelPostsModel::setPosts(const RsGxsChannelGroup& group, std::vector&& posts) { - RsGxsProfiler::Timer prof_timer; - preMods(); initEmptyHierarchy(); @@ -542,15 +539,11 @@ void RsGxsChannelPostsModel::setPosts(const RsGxsChannelGroup& group, std::vecto // rather than deep copying every post (and every thumbnail) one more time. mPosts = std::move(posts); - const long prof_copy_ms = prof_timer.lap(); - // update_posts() already sorted the array in its loader thread. Kept as a // cheap safety net for any other caller: on already ordered input this only // costs comparisons, no element is moved. std::sort(mPosts.begin(),mPosts.end()); - const long prof_sort_ms = prof_timer.lap(); - mFilteredPosts.reserve(mPosts.size()); for(uint32_t i=0;i channelIds; @@ -609,8 +591,6 @@ void RsGxsChannelPostsModel::update_posts(const RsGxsGroupId& group_id) RsGxsChannelGroup group = groups[0]; - const long prof_grpinfo_ms = prof_timer.lap(); - std::vector *posts = new std::vector(); // We use the heap because the arrays need to be stored accross async std::vector comments ; std::vector votes ; @@ -621,23 +601,12 @@ void RsGxsChannelPostsModel::update_posts(const RsGxsGroupId& group_id) return; } - const long prof_content_ms = prof_timer.lap(); - // Sort here rather than in setPosts(): setPosts() runs in the Qt thread, // where sorting a few thousand posts is a visible freeze. The model only // ever displays a sorted array, so the order may as well be established // in this loader thread. std::sort(posts->begin(),posts->end()); - const long prof_sort_ms = prof_timer.ms(); - const long prof_total_ms = prof_grpinfo_ms + prof_content_ms + prof_sort_ms; - - RS_GXS_PROF( prof_total_ms, - "update_posts grp=" << group_id << " posts=" << posts->size() - << " getChannelsInfo=" << prof_grpinfo_ms << "ms" - << " getChannelAllContent=" << prof_content_ms << "ms" - << " sort=" << prof_sort_ms << "ms" - << " service_total=" << prof_total_ms << "ms" ); #ifdef DEBUG_CHANNEL_MODEL std::cerr << "Got channel all content for channel " << group_id << std::endl; std::cerr << " posts : " << posts->size() << std::endl;