From 7d612ab344a7f03cbb4961bc39b88b79874ac540 Mon Sep 17 00:00:00 2001 From: John McLear Date: Sat, 16 May 2026 08:48:41 +0100 Subject: [PATCH] perf: avoid throw-as-control-flow in SessionManager hot path (#7756) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CPU profile of the SUT at the 100-400 author dive sweep (load-test workflow run 25956384097) attributed about 6% of total process CPU to the throw + catch around getSessionInfo: - ~1.82% to `new CustomError('sessionID does not exist', 'apierror')` construction (stack trace capture) - ~4.12% downstream, via the catch block's `console.debug(...)` routed through log4js -> sendToListeners -> sendLogEventToAppender Both call sites (`findAuthorID` on every CLIENT_READY, and `listSessionsWithDBKey` on session listing) immediately caught `apierror` and discarded it. The public `exports.getSessionInfo` contract still has to throw for the HTTP API (returning code:1 for missing sessionID), so introduce a private `getSessionInfoOrNull` helper that returns null and have the hot-path callers use it directly. `exports.getSessionInfo` is kept as a thin wrapper that preserves the existing throw semantics. No behaviour change for the HTTP API — sessionsAndGroups.ts test file (32 cases, including "getSessionInfo of deleted session" expecting code:1) passes unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/node/db/SessionManager.ts | 42 +++++++++++++++++------------------ 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/src/node/db/SessionManager.ts b/src/node/db/SessionManager.ts index b8b1b2562dc..e2f785d82e1 100644 --- a/src/node/db/SessionManager.ts +++ b/src/node/db/SessionManager.ts @@ -62,18 +62,8 @@ exports.findAuthorID = async (groupID:string, sessionCookie: string) => { * Also, see #3820. */ const sessionIDs = sessionCookie.replace(/^"|"$/g, '').split(','); - const sessionInfoPromises = sessionIDs.map(async (id) => { - try { - return await exports.getSessionInfo(id); - } catch (err:any) { - if (err.message === 'sessionID does not exist') { - console.debug(`SessionManager getAuthorID: no session exists with ID ${id}`); - } else { - throw err; - } - } - return undefined; - }); + const sessionInfoPromises = sessionIDs.map(async (id) => + (await getSessionInfoOrNull(id)) || undefined); const now = Math.floor(Date.now() / 1000); const isMatch = (si: { groupID: string; @@ -163,9 +153,20 @@ exports.createSession = async (groupID: string, authorID: string, validUntil: nu * @param {String} sessionID The id of the session * @return {Promise} the sessioninfos */ +// Non-throwing variant for hot-path callers. Hot path uses +// `findAuthorID` on every CLIENT_READY and `listSessionsWithDBKey` on +// session listing; both wrap getSessionInfo in try/catch and discard +// "sessionID does not exist" CustomError. Profiling against develop at +// 100-400 author sweep (ether/etherpad#7756) attributed ~6% of total +// CPU to that throw+catch pair: ~1.8% to CustomError construction and +// ~4% to the cascading `console.debug` call routed through log4js. A +// null return collapses the cost to a single nullable check. +const getSessionInfoOrNull = async (sessionID: string) => + await db.get(`session:${sessionID}`); + exports.getSessionInfo = async (sessionID:string) => { // check if the database entry of this session exists - const session = await db.get(`session:${sessionID}`); + const session = await getSessionInfoOrNull(sessionID); if (session == null) { // session does not exist @@ -250,15 +251,12 @@ const listSessionsWithDBKey = async (dbkey: string) => { // iterate through the sessions and get the sessioninfos for (const sessionID of Object.keys(sessions || {})) { - try { - sessions[sessionID] = await exports.getSessionInfo(sessionID); - } catch (err:any) { - if (err.name === 'apierror') { - console.warn(`Found bad session ${sessionID} in ${dbkey}`); - sessions[sessionID] = null; - } else { - throw err; - } + const info = await getSessionInfoOrNull(sessionID); + if (info == null) { + console.warn(`Found bad session ${sessionID} in ${dbkey}`); + sessions[sessionID] = null; + } else { + sessions[sessionID] = info; } }