Skip to content

Commit 19a9eee

Browse files
claudeKevLehman
authored andcommitted
perf: quick wins on hot paths without behavior changes
- Random.id/secret (server): generate all randomness with a single crypto.randomBytes call instead of one per character (~15x faster, identical output distribution) - Markdown original/filtered parsers: hoist constant regexes and cache the schemes-derived link regexes instead of recompiling per message - MentionsParser: cache user/channel mention regexes keyed on the configured pattern; avoid a duplicate channels.find per mention - AutoTranslate tokenizers: compile constant regexes once at module load - MentionsServer: skip the empty channel-mention DB query on messages with no channel mentions; fetch the room member count at most once per message when handling @all/@here - API response shaping: replace O(n*m) Array.find/includes scans with Map/Set lookups (channels/groups/im files, im.members, browseChannels, Team.findBySubscribedUserIds, LDAP role sync) - Replace quadratic reduce+spread accumulators with Object.assign, Object.fromEntries and flatMap (i18n namespace merge, license v2->v3 conversion, settings $unset build, GenericMenu items) - Avoid repeated settings.get calls in parseUrlsInMessage and getMarkdownConfig Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ETnjqyMsf3b8LAeQ8fYCbG
1 parent fdd4ed7 commit 19a9eee

18 files changed

Lines changed: 179 additions & 91 deletions

File tree

.changeset/perf-quick-wins.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
'@rocket.chat/random': patch
3+
'@rocket.chat/i18n': patch
4+
'@rocket.chat/license': patch
5+
'@rocket.chat/ui-client': patch
6+
'@rocket.chat/meteor': patch
7+
---
8+
9+
Improves performance of several hot code paths without changing behavior: generates random IDs with a single `crypto.randomBytes` call instead of one per character, caches constant regular expressions used by the markdown/mention/autotranslate parsers instead of recompiling them for every message, skips the channel-mention database query for messages without channel mentions, deduplicates the room member count query when a message contains both `@all` and `@here`, and replaces linear array scans and spread-accumulators with Map/Set lookups in API response shaping (files, DM members, directory search and team listing).

apps/meteor/app/markdown/lib/parser/filtered/filtered.js

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
import { getFilteredLinkRegexes } from '../linkRegexes';
2+
3+
const inlineCodeRegex = /`([^`\r\n]+)\`/gm;
4+
15
/**
26
* Filter markdown tags in message
37
* Use case: notifications
@@ -10,21 +14,19 @@ export const filtered = (
1014
},
1115
) => {
1216
const schemes = (options.supportSchemesForLink || 'http,https').split(',').join('|');
17+
const linkRegexes = getFilteredLinkRegexes(schemes);
1318

1419
// Remove block code backticks
1520
message = message.replace(/```/g, '');
1621

1722
// Remove inline code backticks
18-
message = message.replace(new RegExp(/`([^`\r\n]+)\`/gm), (match) => match.substr(1, match.length - 2));
23+
message = message.replace(inlineCodeRegex, (match) => match.substr(1, match.length - 2));
1924

2025
// Filter [text](url), ![alt_text](image_url)
21-
message = message.replace(new RegExp(`!?\\[([^\\]]+)\\]\\((?:${schemes}):\\/\\/[^\\)]+\\)`, 'gm'), (match, title) => title);
26+
message = message.replace(linkRegexes.link, (match, title) => title);
2227

2328
// Filter <http://link|Text>
24-
message = message.replace(
25-
new RegExp(`(?:<|&lt;)(?:${schemes}):\\/\\/[^\\|]+\\|(.+?)(?=>|&gt;)(?:>|&gt;)`, 'gm'),
26-
(match, title) => title,
27-
);
29+
message = message.replace(linkRegexes.pipedLink, (match, title) => title);
2830

2931
// Filter headings
3032
message = message.replace(
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import mem from 'mem';
2+
3+
// The link regexes only vary with the configured URL schemes, which come from
4+
// a setting that rarely changes; cache the compiled regexes per schemes value
5+
// instead of recompiling them for every message.
6+
//
7+
// The two parsers intentionally use different patterns: `filtered` only strips
8+
// markup (no URL capture, URLs may contain `)` escapes) while `original`
9+
// renders it (captures the URL, splits image from link).
10+
11+
export const getMarkdownLinkRegexes = mem((schemes) => ({
12+
image: new RegExp(`!\\[([^\\]]+)\\]\\(((?:${schemes}):\\/\\/[^\\s]+)\\)`, 'gm'),
13+
link: new RegExp(`\\[([^\\]]+)\\]\\(((?:${schemes}):\\/\\/[^\\s]+)\\)`, 'gm'),
14+
pipedLink: new RegExp(`(?:<|&lt;)((?:${schemes}):\\/\\/[^\\|]+)\\|(.+?)(?=>|&gt;)(?:>|&gt;)`, 'gm'),
15+
}));
16+
17+
export const getFilteredLinkRegexes = mem((schemes) => ({
18+
link: new RegExp(`!?\\[([^\\]]+)\\]\\((?:${schemes}):\\/\\/[^\\)]+\\)`, 'gm'),
19+
pipedLink: new RegExp(`(?:<|&lt;)(?:${schemes}):\\/\\/[^\\|]+\\|(.+?)(?=>|&gt;)(?:>|&gt;)`, 'gm'),
20+
}));

apps/meteor/app/markdown/lib/parser/original/markdown.js

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { addAsToken, isToken, validateAllowedTokens } from './token';
2+
import { getMarkdownLinkRegexes } from '../linkRegexes';
23

34
const validateUrl = (url, message) => {
45
// Don't render markdown inside links
@@ -31,7 +32,11 @@ const getTextWrapper = (marker, tagName) => (textPrepend, wrappedText, textAppen
3132

3233
const getRegexReplacer = (replaceFunction, getRegex) => (marker, tagName) => {
3334
const wrapper = getTextWrapper(marker, tagName);
34-
return (msg) => msg.replace(getRegex(marker), (...args) => replaceFunction(wrapper, ...args));
35+
// The regex only depends on the marker, so build it once when the parser is
36+
// created instead of on every message. Safe with /g because
37+
// String.prototype.replace resets lastIndex on each call.
38+
const regex = getRegex(marker);
39+
return (msg) => msg.replace(regex, (...args) => replaceFunction(wrapper, ...args));
3540
};
3641

3742
const getParserWithCustomMarker = getRegexReplacer(
@@ -68,6 +73,7 @@ const parseNotEscaped = (message, { supportSchemesForLink, headers, rootUrl }) =
6873
}
6974

7075
const schemes = (supportSchemesForLink || '').split(',').join('|');
76+
const linkRegexes = getMarkdownLinkRegexes(schemes);
7177

7278
if (headers) {
7379
// Support # Text for h1
@@ -130,7 +136,7 @@ const parseNotEscaped = (message, { supportSchemesForLink, headers, rootUrl }) =
130136
msg = msg.replace(/<\/blockquote>\n<blockquote/gm, '</blockquote><blockquote');
131137

132138
// Support ![alt text](http://image url)
133-
msg = msg.replace(new RegExp(`!\\[([^\\]]+)\\]\\(((?:${schemes}):\\/\\/[^\\s]+)\\)`, 'gm'), (match, title, url) => {
139+
msg = msg.replace(linkRegexes.image, (match, title, url) => {
134140
if (!validateUrl(url, message)) {
135141
return match;
136142
}
@@ -148,7 +154,7 @@ const parseNotEscaped = (message, { supportSchemesForLink, headers, rootUrl }) =
148154
});
149155

150156
// Support [Text](http://link)
151-
msg = msg.replace(new RegExp(`\\[([^\\]]+)\\]\\(((?:${schemes}):\\/\\/[^\\s]+)\\)`, 'gm'), (match, title, url) => {
157+
msg = msg.replace(linkRegexes.link, (match, title, url) => {
152158
if (!validateUrl(url, message)) {
153159
return match;
154160
}
@@ -168,7 +174,7 @@ const parseNotEscaped = (message, { supportSchemesForLink, headers, rootUrl }) =
168174
});
169175

170176
// Support <http://link|Text>
171-
msg = msg.replace(new RegExp(`(?:<|&lt;)((?:${schemes}):\\\/\\\/[^\\|]+)\\|(.+?)(?=>|&gt;)(?:>|&gt;)`, 'gm'), (match, url, title) => {
177+
msg = msg.replace(linkRegexes.pipedLink, (match, url, title) => {
172178
if (!validateUrl(url, message)) {
173179
return match;
174180
}

apps/meteor/app/mentions/lib/MentionsParser.ts

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,34 @@ export class MentionsParser {
4747
this.roomTemplate = roomTemplate;
4848
}
4949

50-
get userMentionRegex() {
51-
return new RegExp(`(^|\\s|>)@(${this.pattern()}(@(${this.pattern()}))?(:([0-9a-zA-Z-_.]+))?)`, 'gm');
50+
// The mention regexes only depend on the configured pattern (a setting that
51+
// rarely changes), but these getters are hit multiple times per message;
52+
// cache the compiled regexes and rebuild only when the pattern changes.
53+
// Sharing the instances is safe because they're only used with
54+
// String.prototype.replace/match, which reset lastIndex on each call.
55+
private cachedPattern: string | undefined;
56+
57+
private cachedUserMentionRegex: RegExp | undefined;
58+
59+
private cachedChannelMentionRegex: RegExp | undefined;
60+
61+
private updateRegexCache() {
62+
const pattern = this.pattern();
63+
if (pattern !== this.cachedPattern || !this.cachedUserMentionRegex || !this.cachedChannelMentionRegex) {
64+
this.cachedPattern = pattern;
65+
this.cachedUserMentionRegex = new RegExp(`(^|\\s|>)@(${pattern}(@(${pattern}))?(:([0-9a-zA-Z-_.]+))?)`, 'gm');
66+
this.cachedChannelMentionRegex = new RegExp(`(^|\\s|>)#(${pattern}(@(${pattern}))?)`, 'gm');
67+
}
68+
}
69+
70+
get userMentionRegex(): RegExp {
71+
this.updateRegexCache();
72+
return this.cachedUserMentionRegex as RegExp;
5273
}
5374

54-
get channelMentionRegex() {
55-
return new RegExp(`(^|\\s|>)#(${this.pattern()}(@(${this.pattern()}))?)`, 'gm');
75+
get channelMentionRegex(): RegExp {
76+
this.updateRegexCache();
77+
return this.cachedChannelMentionRegex as RegExp;
5678
}
5779

5880
replaceUsers = (msg: string, { mentions, temp }: IMessage, me: string) =>
@@ -103,18 +125,14 @@ export class MentionsParser {
103125

104126
replaceChannels = (msg: string, { temp, channels }: IMessage) =>
105127
msg.replace(/&#39;/g, "'").replace(this.channelMentionRegex, (match, prefix, mention) => {
106-
if (
107-
!temp &&
108-
!channels?.find((c) => {
109-
return c.name === mention;
110-
})
111-
) {
112-
return match;
113-
}
114-
115128
const channel = channels?.find(({ name }) => {
116129
return name === mention;
117130
});
131+
132+
if (!temp && !channel) {
133+
return match;
134+
}
135+
118136
const reference = channel ? channel._id : mention;
119137
return this.roomTemplate({ prefix, reference, channel, mention });
120138
});

apps/meteor/server/api/lib/addUserToFileObj.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,10 @@ export async function addUserToFileObj(files: IUpload[]): Promise<(IUpload & { u
77
const uids = files.map(({ userId }) => userId).filter(isString);
88

99
const users = await Users.findByIds(uids, { projection: { name: 1, username: 1 } }).toArray();
10+
const usersById = new Map(users.map((user) => [user._id, user]));
1011

1112
return files.map((file) => {
12-
const user = users.find(({ _id: userId }) => file.userId && userId === file.userId);
13+
const user = file.userId ? usersById.get(file.userId) : undefined;
1314
if (!user) {
1415
return file;
1516
}

apps/meteor/server/api/v1/im.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -590,8 +590,10 @@ const dmMembersAction = <Path extends string>(_path: Path): TypedAction<typeof d
590590
{ projection: { u: 1, status: 1, ts: 1, roles: 1 } },
591591
).toArray();
592592

593+
const subsByUserId = new Map(subs.map((sub) => [sub.u._id, sub]));
594+
593595
const membersWithSubscriptionInfo = members.map((member) => {
594-
const sub = subs.find((sub) => sub.u._id === member._id);
596+
const sub = subsByUserId.get(member._id);
595597

596598
const { u: _u, ...subscription } = sub || {};
597599

apps/meteor/server/lib/autotranslate/autotranslate.ts

Lines changed: 35 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,14 @@ import { notifyOnMessageChange } from '../notifyListener';
1919

2020
const translationLogger = new Logger('AutoTranslate');
2121

22+
// Constant tokenizer patterns, compiled once instead of on every message.
23+
// Safe to share with /g flags because String.prototype.replace resets
24+
// lastIndex on each call.
25+
const urlSchemes = 'http,https';
26+
const markdownLinkRegex = new RegExp(`(!?\\[)([^\\]]+)(\\]\\((?:${urlSchemes}):\\/\\/[^\\)]+\\))`, 'gm');
27+
const pipedLinkRegex = new RegExp(`((?:<|&lt;)(?:${urlSchemes}):\\/\\/[^\\|]+\\|)(.+?)(?=>|&gt;)((?:>|&gt;))`, 'gm');
28+
const wrappedParagraphRegex = new RegExp('^\\s*<p>|</p>\\s*$', 'gm');
29+
2230
const Providers = Symbol('Providers');
2331
const Provider = Symbol('Provider');
2432

@@ -179,47 +187,39 @@ export abstract class AutoTranslate {
179187
tokenizeURLs(message: IMessage): IMessage {
180188
let count = message.tokens?.length || 0;
181189

182-
const schemes = 'http,https';
183-
184190
// Support ![alt text](http://image url) and [text](http://link)
185-
message.msg = message.msg.replace(
186-
new RegExp(`(!?\\[)([^\\]]+)(\\]\\((?:${schemes}):\\/\\/[^\\)]+\\))`, 'gm'),
187-
(_match, pre, text, post) => {
188-
const pretoken = `<i class=notranslate>{${count++}}</i>`;
189-
message.tokens?.push({
190-
token: pretoken,
191-
text: pre,
192-
});
191+
message.msg = message.msg.replace(markdownLinkRegex, (_match, pre, text, post) => {
192+
const pretoken = `<i class=notranslate>{${count++}}</i>`;
193+
message.tokens?.push({
194+
token: pretoken,
195+
text: pre,
196+
});
193197

194-
const posttoken = `<i class=notranslate>{${count++}}</i>`;
195-
message.tokens?.push({
196-
token: posttoken,
197-
text: post,
198-
});
198+
const posttoken = `<i class=notranslate>{${count++}}</i>`;
199+
message.tokens?.push({
200+
token: posttoken,
201+
text: post,
202+
});
199203

200-
return pretoken + text + posttoken;
201-
},
202-
);
204+
return pretoken + text + posttoken;
205+
});
203206

204207
// Support <http://link|Text>
205-
message.msg = message.msg.replace(
206-
new RegExp(`((?:<|&lt;)(?:${schemes}):\\/\\/[^\\|]+\\|)(.+?)(?=>|&gt;)((?:>|&gt;))`, 'gm'),
207-
(_match, pre, text, post) => {
208-
const pretoken = `<i class=notranslate>{${count++}}</i>`;
209-
message.tokens?.push({
210-
token: pretoken,
211-
text: pre,
212-
});
208+
message.msg = message.msg.replace(pipedLinkRegex, (_match, pre, text, post) => {
209+
const pretoken = `<i class=notranslate>{${count++}}</i>`;
210+
message.tokens?.push({
211+
token: pretoken,
212+
text: pre,
213+
});
213214

214-
const posttoken = `<i class=notranslate>{${count++}}</i>`;
215-
message.tokens?.push({
216-
token: posttoken,
217-
text: post,
218-
});
215+
const posttoken = `<i class=notranslate>{${count++}}</i>`;
216+
message.tokens?.push({
217+
token: posttoken,
218+
text: post,
219+
});
219220

220-
return pretoken + text + posttoken;
221-
},
222-
);
221+
return pretoken + text + posttoken;
222+
});
223223

224224
return message;
225225
}
@@ -230,8 +230,7 @@ export abstract class AutoTranslate {
230230
message = Markdown.parseMessageNotEscaped(message);
231231

232232
// Some parsers (e. g. Marked) wrap the complete message in a <p> - this is unnecessary and should be ignored with respect to translations
233-
const regexWrappedParagraph = new RegExp('^\\s*<p>|</p>\\s*$', 'gm');
234-
message.msg = message.msg.replace(regexWrappedParagraph, '');
233+
message.msg = message.msg.replace(wrappedParagraphRegex, '');
235234

236235
for (const [tokenIndex, value] of message.tokens?.entries() ?? []) {
237236
const { token } = value;

apps/meteor/server/lib/messages/parseUrlsInMessage.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,16 @@ import { getMessageUrlRegex } from '../../../lib/getMessageUrlRegex';
55
import { settings } from '../../settings';
66
import { Markdown } from '../messaging/markdown';
77

8-
const prepareUrl = (url: string, previewUrls: string[] | undefined) => ({
8+
const prepareUrl = (url: string, previewUrls: string[] | undefined, siteUrl: string) => ({
99
url,
1010
meta: {},
11-
...(previewUrls && !previewUrls.includes(url) && !url.includes(settings.get('Site_Url')) && { ignoreParse: true }),
11+
...(previewUrls && !previewUrls.includes(url) && !url.includes(siteUrl) && { ignoreParse: true }),
1212
});
1313

14-
const prepareUrls = (urls: string[], previewUrls?: string[]) => [...new Set(urls)].map((url) => prepareUrl(url, previewUrls));
14+
const prepareUrls = (urls: string[], previewUrls?: string[]) => {
15+
const siteUrl = settings.get<string>('Site_Url');
16+
return [...new Set(urls)].map((url) => prepareUrl(url, previewUrls, siteUrl));
17+
};
1518

1619
export const parseUrlsInMessage = (
1720
message: AtLeast<IMessage, 'msg' | 'md'> & {

apps/meteor/server/lib/messaging/mentions/Mentions.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@ export class MentionsServer extends MentionsParser {
5959
const mentionsAll: { _id: string; username: string }[] = [];
6060
const userMentions = new Set<string>();
6161

62+
// A message may mention both @all and @here; fetch the member count at
63+
// most once per message instead of once per mention.
64+
let totalChannelMembers: number | undefined;
65+
6266
for (const m of mentions) {
6367
let mention: string;
6468
if (m.includes(':')) {
@@ -72,9 +76,13 @@ export class MentionsServer extends MentionsParser {
7276
userMentions.add(mention);
7377
continue;
7478
}
75-
if (this.messageMaxAll() > 0 && (await this.getTotalChannelMembers(rid)) > this.messageMaxAll()) {
76-
await this.onMaxRoomMembersExceeded({ sender, rid });
77-
continue;
79+
const messageMaxAll = this.messageMaxAll();
80+
if (messageMaxAll > 0) {
81+
totalChannelMembers ??= await this.getTotalChannelMembers(rid);
82+
if (totalChannelMembers > messageMaxAll) {
83+
await this.onMaxRoomMembersExceeded({ sender, rid });
84+
continue;
85+
}
7886
}
7987
mentionsAll.push({
8088
_id: mention,
@@ -96,6 +104,12 @@ export class MentionsServer extends MentionsParser {
96104
}
97105

98106
async convertMentionsToChannels(channels: string[]): Promise<Pick<IRoom, '_id' | 'name' | 'fname' | 'federated'>[]> {
107+
// Most messages don't mention any channel; skip the (empty) database
108+
// query entirely in that case. An empty $in matches nothing anyway.
109+
if (channels.length === 0) {
110+
return [];
111+
}
112+
99113
return this.getChannels(channels.map((c) => (c.startsWith('#') ? c.substring(1) : c)));
100114
}
101115

0 commit comments

Comments
 (0)