mirror of
https://github.com/RetroShare/RetroShare.git
synced 2026-09-12 19:50:17 +05:00
Merge pull request #3274 from jolavillette/fix/gxs-mark-all-read-nofreeze
Fix UI freeze on "mark all as read/unread" for forums, channels and boards
This commit is contained in:
commit
682cde9fbd
@ -79,6 +79,31 @@ void RsPostedPostsModel::handleEvent_main_thread(std::shared_ptr<const RsEvent>
|
||||
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<RsGxsMessageId> ids(e->mPostedMsgIds.begin(),e->mPostedMsgIds.end());
|
||||
|
||||
for(uint32_t i=0;i<mPosts.size();++i)
|
||||
if(ids.count(mPosts[i].mMeta.mMsgId))
|
||||
{
|
||||
if(e->mPostedMsgsRead)
|
||||
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!
|
||||
@ -737,20 +762,38 @@ void RsPostedPostsModel::createPostsArray(std::vector<RsPostedPost>& 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<RsGxsGrpMsgIdPair> pairs;
|
||||
std::vector<RsGxsMessageId> msgIds;
|
||||
msgIds.reserve(mPosts.size());
|
||||
|
||||
for(uint32_t i=0;i<mPosts.size();++i)
|
||||
pairs.push_back(RsGxsGrpMsgIdPair(mPosts[i].mMeta.mGroupId,mPosts[i].mMeta.mMsgId));
|
||||
{
|
||||
bool post_status = !(IS_MSG_UNREAD(mPosts[i].mMeta.mMsgStatus) || IS_MSG_NEW(mPosts[i].mMeta.mMsgStatus));
|
||||
|
||||
// Call blocking API
|
||||
if(post_status != read)
|
||||
msgIds.push_back(mPosts[i].mMeta.mMsgId);
|
||||
}
|
||||
|
||||
for(auto& p:pairs)
|
||||
RsThread::async([read,p]()
|
||||
if(!msgIds.empty())
|
||||
{
|
||||
// Hand the id list over through a pointer: the lambda capture would
|
||||
// otherwise deep copy it.
|
||||
auto* ids = new std::vector<RsGxsMessageId>(std::move(msgIds));
|
||||
|
||||
RsThread::async([boardId=mPostedGroup.mMeta.mGroupId, ids, read]()
|
||||
{
|
||||
rsPosted->setPostReadStatus(p,read);
|
||||
rsPosted->setPostReadStatus(boardId, *ids, read);
|
||||
delete ids;
|
||||
} );
|
||||
}
|
||||
|
||||
// The model is updated when the resulting READ_STATUS_CHANGED event comes
|
||||
// back (see handleEvent_main_thread()), through the same path as a batch
|
||||
// initiated by any other frontend (e.g. the webUI).
|
||||
}
|
||||
void RsPostedPostsModel::setMsgReadStatus(const QModelIndex& i,bool read_status)
|
||||
{
|
||||
|
||||
@ -652,13 +652,20 @@ void RsGxsChannelPostsModel::setAllMsgReadStatus(bool read_status)
|
||||
pairs.push_back(RsGxsGrpMsgIdPair(mPosts[i].mMeta.mGroupId,mPosts[i].mMeta.mMsgId));
|
||||
}
|
||||
|
||||
// 2 - then call the async methods
|
||||
// 2 - then persist them all in a single background batch: one thread and one
|
||||
// event for the whole set, instead of one detached thread (and one
|
||||
// event) per post, which froze the UI on large channels.
|
||||
|
||||
for(uint32_t i=0;i<pairs.size();++i)
|
||||
RsThread::async([p=pairs[i], read_status]() // use async because each markRead() waits for the token to complete in order to properly acknowledge it.
|
||||
std::vector<RsGxsMessageId> 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.
|
||||
|
||||
@ -759,8 +759,15 @@ void RsGxsForumModel::setPosts(const RsGxsForumGroup& group, const std::vector<F
|
||||
{
|
||||
preMods();
|
||||
|
||||
// Populate the model *before* ending the reset: beginResetModel() must bracket
|
||||
// the actual data swap, otherwise endResetModel() fires while mPosts still holds
|
||||
// the previous hierarchy and the view/proxy caches a stale row->source 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::vector<F
|
||||
debug_dump();
|
||||
#endif
|
||||
|
||||
int count = 0;
|
||||
if(mTreeMode == TREE_MODE_FLAT)
|
||||
count = mPosts.size();
|
||||
else
|
||||
count = mPosts[0].mChildren.size();
|
||||
|
||||
if(count>0)
|
||||
{
|
||||
beginInsertRows(QModelIndex(),0,count-1);
|
||||
endInsertRows();
|
||||
}
|
||||
endResetModel();
|
||||
|
||||
postMods();
|
||||
emit forumLoaded();
|
||||
@ -871,19 +868,71 @@ 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<RsGxsMessageId> changed_msgs;
|
||||
uint32_t changed_entries = 0;
|
||||
recursSetMsgReadStatus(entry,read_status,with_children,changed_msgs,changed_entries) ;
|
||||
|
||||
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 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())
|
||||
{
|
||||
// Hand the (possibly long) id list over through a pointer: the lambda
|
||||
// capture would otherwise deep copy it.
|
||||
auto* msgs = new std::vector<RsGxsMessageId>(std::move(changed_msgs));
|
||||
|
||||
for(QModelIndex j = i.parent(); j.isValid(); j = j.parent())
|
||||
{
|
||||
emit dataChanged(j, j.sibling(j.row(), COLUMN_THREAD_NB_COLUMNS - 1));
|
||||
}
|
||||
RsThread::async( [grpId=mForumGroup.mMeta.mGroupId,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
|
||||
// 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)
|
||||
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 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)
|
||||
void RsGxsForumModel::recursSetMsgReadStatus(ForumModelIndex i,bool read_status,bool with_children,std::vector<RsGxsMessageId>& changed_msgs,uint32_t& changed_entries)
|
||||
{
|
||||
uint32_t newStatus = (read_status ? mPosts[i].mMsgStatus & ~static_cast<int>(GXS_SERV::GXS_MSG_STATUS_GUI_UNREAD)
|
||||
: mPosts[i].mMsgStatus | static_cast<int>(GXS_SERV::GXS_MSG_STATUS_GUI_UNREAD));
|
||||
@ -894,36 +943,28 @@ 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) ;
|
||||
|
||||
// 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<mPosts[i].mChildren.size();++j)
|
||||
recursSetMsgReadStatus(mPosts[i].mChildren[j],read_status,with_children);
|
||||
recursSetMsgReadStatus(mPosts[i].mChildren[j],read_status,with_children,changed_msgs,changed_entries);
|
||||
}
|
||||
|
||||
void RsGxsForumModel::recursUpdateReadStatusAndTimes(ForumModelIndex i,bool& has_unread_below,bool& has_read_below)
|
||||
|
||||
@ -165,7 +165,7 @@ private:
|
||||
void setForumMessageSummary(const std::vector<RsGxsForumMsg>& 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<RsGxsMessageId>& changed_msgs,uint32_t& changed_entries);
|
||||
|
||||
static void generateMissingItem(const RsGxsMessageId &msgId,ForumModelPostEntry& entry);
|
||||
static ForumModelIndex addEntry(std::vector<ForumModelPostEntry>& posts,const ForumModelPostEntry& entry,ForumModelIndex parent);
|
||||
|
||||
@ -23,6 +23,7 @@
|
||||
#include <QKeyEvent>
|
||||
#include <QScrollBar>
|
||||
#include <QPainter>
|
||||
#include <QTimer>
|
||||
|
||||
#include "util/qtthreadsutils.h"
|
||||
#include "util/misc.h"
|
||||
@ -253,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;
|
||||
@ -402,21 +410,17 @@ void GxsForumThreadWidget::handleEvent_main_thread(std::shared_ptr<const RsEvent
|
||||
case RsForumEventCode::PINNED_POSTS_CHANGED:
|
||||
case RsForumEventCode::SYNC_PARAMETERS_UPDATED:
|
||||
if(e->mForumGroupId == mForumGroup.mMeta.mGroupId)
|
||||
updateDisplay(true);
|
||||
scheduleForumReload();
|
||||
break;
|
||||
|
||||
case RsForumEventCode::SUBSCRIBE_STATUS_CHANGED:
|
||||
if(e->mForumGroupId == 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;
|
||||
@ -424,6 +428,16 @@ void GxsForumThreadWidget::handleEvent_main_thread(std::shared_ptr<const RsEvent
|
||||
}
|
||||
}
|
||||
|
||||
void GxsForumThreadWidget::scheduleForumReload()
|
||||
{
|
||||
// Coalesce bursts of incoming events into a single reload. 300 ms is short
|
||||
// enough to feel immediate yet long enough to absorb a whole sync batch, so
|
||||
// the expensive updateForum()/setPosts() cycle runs once instead of once per
|
||||
// post. Restarting the timer on every event pushes the reload back until the
|
||||
// events settle.
|
||||
mDeferredReloadTimer->start(300);
|
||||
}
|
||||
|
||||
void GxsForumThreadWidget::showForumInfo()
|
||||
{
|
||||
mThreadId.clear();
|
||||
@ -596,6 +610,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())
|
||||
@ -1463,7 +1481,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)
|
||||
@ -1475,11 +1492,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()
|
||||
|
||||
@ -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<const RsEvent> 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
|
||||
|
||||
Loading…
Reference in New Issue
Block a user