GXS: warm up the message meta caches with one scan instead of one query per group - #355
Closed
jolavillette wants to merge 7 commits into
Closed
GXS: warm up the message meta caches with one scan instead of one query per group#355jolavillette wants to merge 7 commits into
jolavillette wants to merge 7 commits into
Conversation
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>
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>
…message 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>
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>
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>
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>
…ry per group
Reading the meta of a whole group runs SELECT ... WHERE grpId=?, which
INDEX_MESSAGES_GRPID serves with one row lookup per message. Since the
payload blob lives in the same row, those lookups are scattered over the
whole file: warming up the cache of N groups costs N passes of random
I/O over a database that is hundreds of megabytes.
When more than one group still needs a cold full read, read the meta of
every message in a single sequential scan instead and fill every
per-group cache from it. The file is then read in physical order, and
the cost no longer grows with the number of groups.
Measured on a synthetic database of the same shape and size as a real
gxsforums_db (235 MB, 23 KB rows, 20 groups), cold cache:
20 per-group queries 29449 ms
one sequential scan 2146 ms 13.7x
and the scan does not get more expensive as groups are added, where the
per-group path grows linearly with them. On a node subscribed to
hundreds of forums this is the difference between tens of seconds of
startup and a fixed couple of seconds.
Nothing else changes: same columns, same cache contents, same values
returned. Callers and public API are untouched, and no database schema
or format is modified.
Trade-off: the scan fills the cache for groups that were not requested
yet. That is the same memory the cache reaches as soon as those groups
are browsed, but it is reached up front rather than progressively.
The scan reports itself through the existing opt-in profiler, so the
gain is verifiable on a real profile rather than taken on trust:
GXS-PROF loadAllMsgMetaInOneScan db=gxsforums_db groups=571 metas=48213 in 2100ms
Stacked on the channel loading branch: both reshape the same function,
and this one reuses the profiler introduced there.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jolavillette
force-pushed
the
perf/gxs-meta-single-scan
branch
from
July 29, 2026 17:39
e229299 to
bccd9d1
Compare
Contributor
Author
|
Closing: measured on a real profile, this makes things worse, and my benchmark was wrong. On a real A full table scan is only sequential if the rows are laid out in physical order. A forum database is constantly purged (storage period), so freed pages get reused out of order and the large payload blobs live in overflow chains scattered across the file. The scan then degenerates into random I/O, which is what 2.9 MB/s means. My benchmark showed 13.7x because the synthetic database had just been written in one go and was perfectly contiguous -- it did not represent a long-lived database. That is a flaw in the measurement, not a detail. Leaving this closed rather than tuning the trigger: the premise itself does not hold. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds on #351 (same function, and reuses the profiler introduced there). One commit on top of it.
Problem
Reading the meta of a whole group runs
SELECT ... WHERE grpId=?, whichINDEX_MESSAGES_GRPIDserves with one row lookup per message. The payload blob lives in the same row, so those lookups are scattered over the whole file. Warming up the cache of N groups therefore costs N passes of random I/O over a database that is hundreds of megabytes, and the cost grows linearly with the number of groups.On a node subscribed to hundreds of forums this shows up as tens of seconds before the unread counters settle at startup, with the biggest groups taking seconds each on their own (26 s measured for a 9573-message forum on a spinning disk).
Fix
When more than one group still needs a cold full read, read the meta of every message in a single sequential scan and fill every per-group cache from it. The file is then read in physical order, and warming up all the groups costs one pass whatever their number.
Measured on a synthetic database of the same shape and size as a real
gxsforums_db(235 MB, 23 KB rows, 20 groups, cold page cache enforced between runs):The per-group path grows with the number of groups; the scan does not.
Scope
RS_GXS_PROFILE), so the gain is checkable on a real profile:GXS-PROF loadAllMsgMetaInOneScan db=gxsforums_db groups=571 metas=48213 in 2100msTrade-off worth reviewing
The scan fills the cache for groups that were not requested yet. That is the same memory the cache reaches as soon as those groups are browsed, but it is reached up front instead of progressively. The trigger is deliberately conservative (more than one group needing a cold full read, and only once per database), but if you would rather bound it explicitly — by message count, or by only caching the requested groups — say so and I will adjust.