processGrpMetaChanges() wrote each queued group meta update with its own
call to updateGroupMetaData(), i.e. one implicit SQLCipher transaction and
one fsync per entry. One such write was measured at ~1 s, and services
accumulating many updates (typically identity usage stamps at startup,
~3700 identity groups) froze their tick thread for as long as 86 s while
draining the queue, blocking all other GXS activity of the service through
mGenMtx.
Mirror the existing message-side batching: add a vector variant of
updateGroupMetaData() that wraps all row updates in a single transaction
(one fsync total), and make processGrpMetaChanges() collect the entries
that pass their mask and write them in one call.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both files were listed twice in src/CMakeLists.txt: once (correctly) in
RS_SOURCES, and once again in RS_IMPLEMENTATION_HEADERS right next to
their .h namesake -- a copy/paste slip dating back to the initial CMake
port (449fcbc3).
RS_IMPLEMENTATION_HEADERS does not feed add_library(), so this never
caused a duplicate compilation. It is only consumed by the install()
loop guarded by RS_LIBRETROSHARE_STANDALONE_INSTALL, which was therefore
copying the two .cc files into the installed include directory.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
getChannelAllContent() split the metas into posts and comments/votes on
"mThreadId.isNull() && mParentId.isNull()". Only the first half of that test
carries information: a post never has a thread id, a comment always carries
the id of the post it belongs to, and a vote the id of the post whose comment
is being voted. mParentId proves nothing, since comments written under the old
comment paradigm have a null one.
Reported by csoler on PR #351.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Requested in review: the instrumentation added by the first commit served
to measure the four defects and verify the fixes, but it should not stay
in the optimised code. The probes are removed from rsdataservice,
rsgenexchange and p3gxschannels; convertMsgItems() loses the two timing
out-parameters that only existed to feed them.
The profiler class itself (gxs/rsgxsprofiler.h) is kept, per review, for
future measurement work. Nothing includes it anymore.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Profiling the version filtering added in the previous commit showed the
batched IN(...) retrieval costing a near constant 60 to 95ms per batch
whatever the number of messages it returned: 5 batches for 2203 messages took
454ms, 7 batches for 3127 took 451ms. That is the signature of each batch
walking the whole group, and it made a channel with almost no edited posts
slower than before (272ms in one query against 451ms in seven).
The cause is the query plan. On "grpId=... AND msgId IN (...)" sqlite has no
ANALYZE data, so it estimates an equality on the non unique group index at
about ten rows, against one row per entry of the IN list. The group therefore
looks fifty times more selective than it really is -- it matches every message
of the channel -- and gets picked, so every batch scans the group and filters.
Select on the message id alone. It is the table's primary key, hence unique
table wide, so the result is identical while sqlite can seek straight into the
implicit unique index. locked_retrieveMessages() takes an optional expected
group and drops anything else, so a caller mixing groups still cannot get
foreign messages attributed to the wrong one.
Reading most of a group by id remains slower than scanning it once, whatever
the index. So getChannelAllContent() now only filters versions out when they
are worth filtering: past MAX_READ_RATIO_FOR_VERSION_FILTERING of the group's
messages it requests the whole group and resolves versions with sortPosts() as
before. The profiling line reports which path was taken.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A channel keeps every version of every edited post. Only the latest of each
chain is ever shown: sortPosts() read them all, then discarded the superseded
ones keeping just their ids in mOlderVersions. Profiling a real channel shows
how much that costs: 6409 messages read for 1931 displayed posts, 195MB of
payload of which roughly two thirds belongs to versions thrown away
immediately -- thumbnails decrypted, deserialised and freed for nothing.
Resolve the version chains on the metas instead. They are small, come from
the meta cache once warm, and sortPostMetas() already works on any type
exposing a RsMsgMetaData. getChannelAllContent() now:
- pulls the group's metas via getContentSummaries()
- splits posts from comments and votes
- runs sortPostMetas() to find the retained version of each chain
- requests message data for those ids only, plus all comments and votes
Since the request now carries an explicit id set, it goes through the batched
IN(...) retrieval added earlier: a few queries instead of one, and the payload
read drops by whatever the edit history weighs.
Two behaviours of sortPosts() have to be reproduced, and applyPostVersions()
does so from the resolved chains:
- comments hang off whichever version was current when they were written, so
they are remapped onto the retained post before being counted, which
replaces the old "add up the counts of all older versions" pass;
- sortPostMetas() normalises mOrigMsgId to the top of the chain, and callers
match edited posts on that value (GUI updateSinglePost), so the normalised
id is carried over to the post that is returned.
The item conversion loop is factored out of getPostData() into
convertMsgItems() so both paths share it; getPostData() itself, still used by
getChannelContent() and the deprecated API, keeps calling sortPosts()
unchanged.
An empty id set means "every message of the group" to the data store, so an
empty channel returns before any request is made rather than asking for
everything.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
RsGxsImage owns a malloc'd buffer and declares a destructor, a copy
constructor and a copy assignment. RsMsgMetaData and RsGxsGenericMsgData
declare destructors too, and so does RsGxsChannelPost. Each of those user
declared destructors suppresses the implicit move operations, so every
std::move() on a post silently resolved to the copy constructor: a malloc
plus memcpy of the thumbnail.
That cost is paid far more often than it looks. A channel's post array is
copied whole at four hand-off points between the store, the service and the
model, none of which reserve, so vector growth copies on top. Worse,
std::sort falls back to copies as well, which for a few thousand posts means
tens of thousands of thumbnail duplications -- and that sort runs in the GUI
thread.
Give RsGxsImage real move operations that steal the buffer, and explicitly
default the copy and move operations of RsMsgMetaData, RsGxsGenericMsgData and
RsGxsChannelPost. The resulting RsGxsChannelPost move constructor is noexcept,
which is what std::vector requires to move rather than copy on reallocation.
Then use them where the arrays are handed over: reserve and move in
getPostData() and sortPosts() instead of copying element by element, and move
the sorted array back into the caller's vector.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
RsDataService::retrieveNxsMsgs() issued one prepared statement per requested
message id. Each one rebuilds the SQL text, runs a full sqlite3_prepare_v2
(SQL parse plus query planner), allocates a cursor and finalizes it -- an
overhead that dominates the actual row lookup, and is paid thousands of times
whenever a request covers a large id set.
Pack the ids into "msgId IN (...)" batches of 500 instead. The message ids are
plain hex strings so they need no escaping, and the batch size keeps both the
generated SQL and sqlite's expression tree small.
The previous commit removed this path for unfiltered whole-group requests;
this one covers everything else: a channel post with its comments, forum
threads, and any filtered request.
retrieveGxsMsgMetaData() still has the same one-query-per-id shape in its
non-empty branch, but there each id is first looked up in the meta cache, so
the remaining queries are only the cache misses. Left alone for now so it can
be measured on its own.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
RsGxsDataAccess::getMsgData() always ran the request through getMsgIdList(),
which walks every message meta of the group and returns the explicit list of
matching message ids. That list was then handed to RsDataService, whose
retrieveNxsMsgs() has two paths: a single "WHERE grpId=..." query when the id
set is empty, and one prepared query per message otherwise.
Since a request for a whole group carries an empty id set precisely to mean
"all messages", expanding it into 6400 explicit ids meant the fast path was
never taken when opening a channel: the store issued 6400 separate
sqlite3_prepare_v2 + step + finalize cycles against an encrypted database,
plus a full preliminary pass over the metas that produced nothing the caller
did not already know.
When none of mStatusMask, mMsgFlagMask, MSG_LATEST, MSG_ORIGMSG or MSG_THREAD
is set, no filtering can occur, so pass the request through untouched. The
resulting message set is identical, the meta pass disappears, and loading a
whole group collapses to a single SQL query.
Requests that do filter are unaffected and keep the previous path.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Loading a channel with a few thousand posts takes ten seconds or more, and
the cost is spread over four layers with no way to tell which one dominates:
SQL retrieval in RsDataService, deserialisation in RsGenExchange, conversion
to service structures in p3GxsChannels, then the model update in the GUI.
Add a small header-only helper (gxs/rsgxsprofiler.h) and instrument those
layers so each reports its own breakdown on one line. Profiling stays off
unless the RS_GXS_PROFILE environment variable is set; its value is a
reporting threshold in milliseconds so only the operations worth looking at
show up (RS_GXS_PROFILE=0 reports everything).
The reported counters are the ones that matter for the known bottlenecks:
number of SQL queries issued and blob volume read in retrieveNxsMsgs, number
of metas walked in retrieveGxsMsgMetaData, mGenMtx wait and deserialisation
time in getMsgData, and the token wait in getChannelAllContent.
No behaviour change: when profiling is disabled the added work is a couple of
steady_clock reads per call and one comparison against a cached threshold.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- rsgds.h: drop the default implementation of the bulk
updateMessageMetaData(), RsDataService is the only implementation.
- channels/forums/posted: wait on every queued token instead of assuming
they complete in submission order (same cost today, safe if the
processing ever gets parallelized). posted had the same pattern as the
two flagged call sites.
- posted: the batched READ_STATUS_CHANGED event now carries the affected
message ids, so consumers can update their view without reloading the
board — including changes initiated by another frontend (e.g. webUI).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Same treatment as the forum bulk markRead(), now for channels and boards, so
their "mark all as read/unread" also persists in a single transaction and emits
a single event instead of one blocking request + one event per post:
- RsGxsChannels / p3GxsChannels: add setMessageReadStatus(channelId, msgIds, read).
- RsPosted / p3Posted: add setPostReadStatus(boardId, msgIds, read).
Both reuse the batched, single-transaction processMsgMetaChanges() path added
for forums, which already benefits every GXS service.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Marking a large forum (thousands of posts) "read" used to be catastrophic:
the GUI issued one blocking markRead() per post, and the GXS backend wrote
each status change to the SQLCipher DB in its own implicit transaction
(one fsync per message). On an 8000-post forum this took over an hour and,
if force-quit, left most posts still unread.
Backend changes:
- RsGeneralDataService: add a batch updateMessageMetaData(vector) overload.
The default loops; RsDataService overrides it to persist the whole batch in
a single transaction, held under one mDbMutex so nothing interleaves.
- RsGenExchange::processMsgMetaChanges(): resolve all status masks first, then
persist every queued change through that single batched transaction.
- RsGxsForums / p3GxsForums: add a bulk markRead(forumId, msgIds, read) that
queues all status changes at once and emits a single event, instead of one
blocking request + one event per message.
"Mark all as read" now persists in a single transaction (one fsync) whatever
the number of posts.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The cleanup() timer used getDirectoryRecursModTime()==0 to decide whether
a friend's cached list is "empty" (5-days retention) or "non-empty"
(60-days). But a tree made of directories only, with no files, still has a
non-zero recursive modification time coming from the directory timestamps.
Such a peer is shown as "Empty" in Friends Files (the GUI uses the root
cumulated file count) yet was kept on the 60-days timer, so it never lined
up with what the user sees.
Decide emptiness from the same signal as the GUI: the cumulated file count
at the root. A peer that shares no file is now treated as empty and pruned
after 5 days offline, matching the "Empty" label. Add a small
getDirectoryCumulatedFileCount() accessor next to the existing timestamp
getters, and fix the "ffline" typo while here.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The inactivity thresholds used to purge a friend's cached shared-file
list carried an extra *24 factor. 86400 is already one day in seconds,
so 60*24*86400 was 1440 days (~4 years) and 5*24*86400 was 120 days
(~4 months) instead of the documented 60 and 5 days.
Consequence: a friend's populated remote directory listing was in
practice never cleaned, even after months offline (only after ~4 years).
Drop the redundant *24 so the values match their comments: 60 days for a
non-empty listing, 5 days for an empty one. Also fix the "remoe" typo.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The CMake build never compiled the experimental GXS "Wiki" (WikiPoos) and
"The Wire" services: their sources were commented out in src/CMakeLists.txt
and no option defined the RS_USE_WIKI / RS_USE_WIRE macros that gate the
service instantiation in rsserver/rsinit.cc and the matching GUI code. As a
result the retroshare-gui RS_GXSWIKIPOS / RS_GXSTHEWIRE blocks could not be
enabled to a working build.
Mirror the qmake "wikipoos" / "gxsthewire" CONFIG switches:
- Add RS_GXSWIKIPOS and RS_GXSTHEWIRE options (default OFF, as in qmake).
- Compile the p3wiki/p3wire services, rswikiitems/rswireitems and the
rswiki.h/rswire.h public headers when the respective option is enabled.
- Define RS_USE_WIKI / RS_USE_WIRE PUBLIC so libretroshare consumers see the
same guard.
Both features stay OFF by default; they are unmaintained and experimental.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- pqissllistener: restore the unconditional "connected to" stderr line
on successful incoming connections, as requested.
- rsgxsnetservice: log the missing-recipients case unconditionally again
(as a one-line RsWarn with the circle id) instead of hiding it behind
NXS_NET_DEBUG_7 — it can be a genuine error signal.
- distributedchat: remove the mutex from logBannedIdentityDrop(): both
callers are in the chat item receiving path, which is single-threaded.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Same information as before (nothing hidden), fewer lines per connection.
Only the success-path step-by-step trace in pqissllistener is touched;
authssl / pqissl outcome lines and every failure line are left as-is.
- continueSSL(): the incoming cert identity was dumped over 4 lines
(ContinueSSL / Got PGP Id / Got SSL Id / Got SSL CN) inside an inverted
"#ifndef DEBUG_LISTENNER" guard (i.e. on by default, and defining the
macro would have hidden it). Collapse to one RsInfo line carrying the
same pgpId / sslId / CN, unconditionally.
- finaliseConnection(): the success path emitted 5 lines (function name,
"checking:", "Found Matching Peer", "Passing to pqissl module", plus a
redundant std::cerr "connected to"). Collapse to one line keeping the
peer id, remote address and outcome. Same for the no-match failure
path: one line keeping peer id, address and reason.
- finaliseAccepts() / isSSLActive(): two pure step markers with no data
were logged at PQL_WARNING; demote to PQL_DEBUG_BASIC so the existing
level filter (default = Warning) drops them, without deleting them.
Net: an incoming successful connection goes from ~11 lines to ~2 in this
file, with every field preserved. Failure diagnostics are unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two more high-volume, non-error log sources, both left behaving exactly
as before (nothing is hidden):
- RNPPGPHandler::initCertificateInfo(): gate the per-key keyring dump
(one "type/Key id/fingerprint" line + one "N signers" line per key)
behind DEBUG_PGP_KEYRING_DUMP, off by default. With a large keyring
this is thousands of lines of pure inventory at every startup. Key
parse errors throw, so gating hides nothing; the "Loaded N public
keys" summary is still printed unconditionally.
- DistributedChatService: a single locally-banned identity can flood a
lobby with thousands of items, and the code logged one WARN per
dropped item. Every item is still dropped (correct, expected
behaviour); logBannedIdentityDrop() now rate-limits the log to one
line when the flood starts plus one summary per identity per 60s
carrying the dropped count. Signature mismatches and other genuine
problems remain logged separately and unconditionally.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
None of these change behavior; they only fix logging level/volume for
conditions that are normal and already handled by the code.
- p3MsgService::loadList(): drop the per-message RsErr() dump of msg.to
(was ~1 ERROR line per stored mail at every startup).
- p3MsgService::locked_checkForDuplicates(): collapse the per-ID
"Duplicate ID ... replaced" warnings into a single summary line per
message box (and one for msgOutgoing). msgId is a uint32, so ID
collisions in a large store are expected and recovered by renumbering;
no need for one WARN per collision (was ~1000 lines at startup).
- p3IdService::cache_store(): gate "No Public Key Found" behind
DEBUG_IDS. Identity data can legitimately arrive before/without its
public key; the caller retries later, so this is not an error.
- RsGxsNetService::encryptSingleNxsItem(): gate "Cannot encrypt
transaction: recipients list not available" behind NXS_NET_DEBUG_7.
The circle membership may simply not be cached yet; it is retried.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
RsGxsDataAccess::getGroupStatistic() looks like a drop-in replacement
that would avoid rebuilding the whole post hierarchy, but its
obsolete-version filter only ever marks the original message obsolete,
so a post edited N times is counted N times and superseded versions
keep their unread flag forever. Tried in the field: the unread counter
stuck at a non-zero value with everything read, and jumped by the
number of stored versions when one post was marked unread.
Leave a warning in place so the shortcut is not attempted again, and
state what the real optimisation is: factoring the version collapsing
out of computeMessagesHierarchy() so both paths share it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
notifyChanges() switches on the raw notification type, but TYPE_PROCESSED
is emitted for two unrelated things: a group meta change (subscription)
and a message meta change (read/unread, keep-forever). The message case
fell into the group case, so every single post marked read or unread was
announced to all clients as a subscription change. The GUI reacts to that
event by re-reading the group data, reloading the whole forum list and
invalidating the News Feed items, which is expensive work multiplied by
every read-status toggle.
Channels and boards are not affected: they only test TYPE_PROCESSED
inside their group-change branch. Read status changes are already
notified by setMessageReadStatus() and markRead().
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
