Skip to content

Commit 3326d7c

Browse files
committed
chat: rewrite getSortedDates / isThread to be vastly more efficient and scalable
- this was easy (with coffee) and an obvious todo.
1 parent 46b1124 commit 3326d7c

File tree

2 files changed

+53
-25
lines changed

2 files changed

+53
-25
lines changed

src/packages/frontend/chat/chat-log.tsx

Lines changed: 50 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -77,11 +77,16 @@ export function ChatLog({
7777

7878
const user_map = useTypedRedux("users", "user_map");
7979
const account_id = useTypedRedux("account", "account_id");
80-
const { dates: sortedDates, numFolded } = useMemo<{
80+
const {
81+
dates: sortedDates,
82+
numFolded,
83+
numChildren,
84+
} = useMemo<{
8185
dates: string[];
8286
numFolded: number;
87+
numChildren;
8388
}>(() => {
84-
const { dates, numFolded } = getSortedDates(
89+
const { dates, numFolded, numChildren } = getSortedDates(
8590
messages,
8691
search,
8792
account_id!,
@@ -97,7 +102,7 @@ export function ChatLog({
97102
: new Date(parseFloat(dates[dates.length - 1])),
98103
);
99104
}, 1);
100-
return { dates, numFolded };
105+
return { dates, numFolded, numChildren };
101106
}, [messages, search, project_id, path, filterRecentH]);
102107

103108
useEffect(() => {
@@ -237,6 +242,7 @@ export function ChatLog({
237242
manualScrollRef,
238243
mode,
239244
selectedDate,
245+
numChildren,
240246
}}
241247
/>
242248
<Composing
@@ -283,21 +289,14 @@ function isPrevMessageSender(
283289
);
284290
}
285291

286-
function isThread(messages: ChatMessages, message: ChatMessageTyped) {
292+
function isThread(
293+
message: ChatMessageTyped,
294+
numChildren: { [date: number]: number },
295+
) {
287296
if (message.get("reply_to") != null) {
288297
return true;
289298
}
290-
291-
// TODO/WARNING!!! This is a linear search
292-
// through all messages to decide if a message is the root of a thread.
293-
// This is VERY BAD and must to be redone at some point, since we call isThread
294-
// on all messages (in getSortedDates), making that algorithm O(n^2),
295-
// which is hideous as the number of messages scales. Instead one must
296-
// use a proper data structure (or even a cache) to track this once
297-
// and for all. It's more complicated but everything needs to be at
298-
// most O(n).
299-
const s = message.get("date").toISOString();
300-
return messages.some((m) => m.get("reply_to") === s);
299+
return (numChildren[message.get("date").valueOf()] ?? 0) > 0;
301300
}
302301

303302
function isFolded(
@@ -323,24 +322,45 @@ export function getSortedDates(
323322
search: string | undefined,
324323
account_id: string,
325324
filterRecentH?: number,
326-
): { dates: string[]; numFolded: number } {
325+
): {
326+
dates: string[];
327+
numFolded: number;
328+
numChildren: { [date: number]: number };
329+
} {
327330
let numFolded = 0;
328331
let m = messages;
329332
if (m == null) {
330-
return { dates: [], numFolded: 0 };
333+
return {
334+
dates: [],
335+
numFolded: 0,
336+
numChildren: {},
337+
};
331338
}
332339

340+
// we assume filterMessages contains complete threads. It does
341+
// right now, but that's an assumption in this function.
333342
m = filterMessages({ messages: m, filter: search, filterRecentH });
334343

344+
// Do a linear pass through all messages to divide into threads, so that
345+
// getSortedDates is O(n) instead of O(n^2) !
346+
const numChildren: { [date: number]: number } = {};
347+
for (const [_, message] of m) {
348+
const parent = message.get("reply_to");
349+
if (parent != null) {
350+
const d = new Date(parent).valueOf();
351+
numChildren[d] = (numChildren[d] ?? 0) + 1;
352+
}
353+
}
354+
335355
const v: [date: number, reply_to: number | undefined][] = [];
336356
for (const [date, message] of m) {
337357
if (message == null) continue;
338358

339359
// If we search for a message, we treat all threads as unfolded
340360
if (!search) {
341-
const is_thread = isThread(messages, message);
342-
const is_folded = isFolded(messages, message, account_id);
343-
const is_thread_body = message.get("reply_to") != null;
361+
const is_thread = isThread(message, numChildren);
362+
const is_folded = is_thread && isFolded(messages, message, account_id);
363+
const is_thread_body = is_thread && message.get("reply_to") != null;
344364
const folded = is_thread && is_folded && is_thread_body;
345365
if (folded) {
346366
numFolded++;
@@ -356,7 +376,7 @@ export function getSortedDates(
356376
}
357377
v.sort(cmpMessages);
358378
const dates = v.map((z) => `${z[0]}`);
359-
return { dates, numFolded };
379+
return { dates, numFolded, numChildren };
360380
}
361381

362382
/*
@@ -465,6 +485,7 @@ export function MessageList({
465485
manualScrollRef,
466486
mode,
467487
selectedDate,
488+
numChildren,
468489
}: {
469490
messages;
470491
account_id;
@@ -481,6 +502,7 @@ export function MessageList({
481502
costEstimate?;
482503
manualScrollRef?;
483504
selectedDate?: string;
505+
numChildren?;
484506
}) {
485507
const virtuosoHeightsRef = useRef<{ [index: number]: number }>({});
486508
const virtuosoScroll = useVirtuosoScrollHook({
@@ -510,9 +532,13 @@ export function MessageList({
510532
return <div style={{ height: "1px" }} />;
511533
}
512534

513-
const is_thread = isThread(messages, message);
514-
const is_folded = isFolded(messages, message, account_id);
515-
const is_thread_body = message.get("reply_to") != null;
535+
// only do threading if numChildren is defined. It's not defined,
536+
// e.g., when viewing past versions via TimeTravel.
537+
const is_thread = numChildren != null && isThread(message, numChildren);
538+
// optimization: only threads can be folded, so don't waste time
539+
// checking on folding state if it isn't a thread.
540+
const is_folded = is_thread && isFolded(messages, message, account_id);
541+
const is_thread_body = is_thread && message.get("reply_to") != null;
516542
const h = virtuosoHeightsRef.current[index];
517543

518544
return (

src/packages/frontend/chat/message.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -904,7 +904,9 @@ export default function Message({
904904
}
905905

906906
function renderFoldedRow() {
907-
if (!is_folded || !is_thread || is_thread_body) return;
907+
if (!is_folded || !is_thread || is_thread_body) {
908+
return;
909+
}
908910

909911
return (
910912
<Col xs={24}>

0 commit comments

Comments
 (0)