Skip to content
This repository was archived by the owner on Jul 8, 2025. It is now read-only.

Commit d653cdf

Browse files
andybalaamtoomore
authored andcommitted
Further improve performance with lots of hidden events (#10353)
* Avoid re-calculating shouldShowEvent in getReadReceiptsByShownEvent * Test that uses a MainGrouper * Cache more calls to shouldShowEvent
1 parent d555739 commit d653cdf

File tree

3 files changed

+263
-65
lines changed

3 files changed

+263
-65
lines changed

src/components/structures/MessagePanel.tsx

Lines changed: 84 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -564,7 +564,7 @@ export default class MessagePanel extends React.Component<IProps, IState> {
564564
/**
565565
* Find the next event in the list, and the next visible event in the list.
566566
*
567-
* @param event - the list of events to look in and whether they are shown
567+
* @param events - the list of events to look in and whether they are shown
568568
* @param i - where in the list we are now
569569
*
570570
* @returns { nextEvent, nextTile }
@@ -578,14 +578,14 @@ export default class MessagePanel extends React.Component<IProps, IState> {
578578
private getNextEventInfo(
579579
events: EventAndShouldShow[],
580580
i: number,
581-
): { nextEvent: MatrixEvent | null; nextTile: MatrixEvent | null } {
581+
): { nextEventAndShouldShow: EventAndShouldShow | null; nextTile: MatrixEvent | null } {
582582
// WARNING: this method is on a hot path.
583583

584-
const nextEvent = i < events.length - 1 ? events[i + 1].event : null;
584+
const nextEventAndShouldShow = i < events.length - 1 ? events[i + 1] : null;
585585

586586
const nextTile = findFirstShownAfter(i, events);
587587

588-
return { nextEvent, nextTile };
588+
return { nextEventAndShouldShow, nextTile };
589589
}
590590

591591
private get pendingEditItem(): string | undefined {
@@ -615,16 +615,16 @@ export default class MessagePanel extends React.Component<IProps, IState> {
615615

616616
let lastShownNonLocalEchoIndex = -1;
617617
for (let i = events.length - 1; i >= 0; i--) {
618-
const { event: mxEv, shouldShow } = events[i];
618+
const { event, shouldShow } = events[i];
619619
if (!shouldShow) {
620620
continue;
621621
}
622622

623623
if (lastShownEvent === undefined) {
624-
lastShownEvent = mxEv;
624+
lastShownEvent = event;
625625
}
626626

627-
if (mxEv.status) {
627+
if (event.status) {
628628
// this is a local echo
629629
continue;
630630
}
@@ -641,20 +641,21 @@ export default class MessagePanel extends React.Component<IProps, IState> {
641641
// to assume that sent receipts are to be shown more often.
642642
this.readReceiptsByEvent = new Map();
643643
if (this.props.showReadReceipts) {
644-
this.readReceiptsByEvent = this.getReadReceiptsByShownEvent();
644+
this.readReceiptsByEvent = this.getReadReceiptsByShownEvent(events);
645645
}
646646

647647
let grouper: BaseGrouper | null = null;
648648

649649
for (let i = 0; i < events.length; i++) {
650-
const { event: mxEv, shouldShow } = events[i];
651-
const eventId = mxEv.getId();
652-
const last = mxEv === lastShownEvent;
653-
const { nextEvent, nextTile } = this.getNextEventInfo(events, i);
650+
const eventAndShouldShow = events[i];
651+
const { event, shouldShow } = eventAndShouldShow;
652+
const eventId = event.getId();
653+
const last = event === lastShownEvent;
654+
const { nextEventAndShouldShow, nextTile } = this.getNextEventInfo(events, i);
654655

655656
if (grouper) {
656-
if (grouper.shouldGroup(mxEv)) {
657-
grouper.add(mxEv);
657+
if (grouper.shouldGroup(eventAndShouldShow)) {
658+
grouper.add(eventAndShouldShow);
658659
continue;
659660
} else {
660661
// not part of group, so get the group tiles, close the
@@ -666,8 +667,15 @@ export default class MessagePanel extends React.Component<IProps, IState> {
666667
}
667668

668669
for (const Grouper of groupers) {
669-
if (Grouper.canStartGroup(this, mxEv) && !this.props.disableGrouping) {
670-
grouper = new Grouper(this, mxEv, prevEvent, lastShownEvent, nextEvent, nextTile);
670+
if (Grouper.canStartGroup(this, eventAndShouldShow) && !this.props.disableGrouping) {
671+
grouper = new Grouper(
672+
this,
673+
eventAndShouldShow,
674+
prevEvent,
675+
lastShownEvent,
676+
nextEventAndShouldShow,
677+
nextTile,
678+
);
671679
break; // break on first grouper
672680
}
673681
}
@@ -677,8 +685,8 @@ export default class MessagePanel extends React.Component<IProps, IState> {
677685
// make sure we unpack the array returned by getTilesForEvent,
678686
// otherwise React will auto-generate keys, and we will end up
679687
// replacing all the DOM elements every time we paginate.
680-
ret.push(...this.getTilesForEvent(prevEvent, mxEv, last, false, nextEvent, nextTile));
681-
prevEvent = mxEv;
688+
ret.push(...this.getTilesForEvent(prevEvent, event, last, false, nextEventAndShouldShow, nextTile));
689+
prevEvent = event;
682690
}
683691

684692
const readMarker = this.readMarkerForEvent(eventId, i >= lastShownNonLocalEchoIndex);
@@ -698,8 +706,8 @@ export default class MessagePanel extends React.Component<IProps, IState> {
698706
mxEv: MatrixEvent,
699707
last = false,
700708
isGrouped = false,
701-
nextEvent?: MatrixEvent,
702-
nextEventWithTile?: MatrixEvent,
709+
nextEvent: EventAndShouldShow | null = null,
710+
nextEventWithTile: MatrixEvent | null = null,
703711
): ReactNode[] {
704712
const ret: ReactNode[] = [];
705713

@@ -745,20 +753,20 @@ export default class MessagePanel extends React.Component<IProps, IState> {
745753
const readReceipts = this.readReceiptsByEvent.get(eventId);
746754

747755
let isLastSuccessful = false;
748-
const isSentState = (s: EventStatus): boolean => !s || s === EventStatus.SENT;
756+
const isSentState = (s: EventStatus | null): boolean => !s || s === EventStatus.SENT;
749757
const isSent = isSentState(mxEv.getAssociatedStatus());
750-
const hasNextEvent = nextEvent && this.shouldShowEvent(nextEvent);
758+
const hasNextEvent = nextEvent?.shouldShow;
751759
if (!hasNextEvent && isSent) {
752760
isLastSuccessful = true;
753-
} else if (hasNextEvent && isSent && !isSentState(nextEvent.getAssociatedStatus())) {
761+
} else if (hasNextEvent && isSent && !isSentState(nextEvent.event.getAssociatedStatus())) {
754762
isLastSuccessful = true;
755763
}
756764

757765
// This is a bit nuanced, but if our next event is hidden but a future event is not
758766
// hidden then we're not the last successful.
759767
if (
760768
nextEventWithTile &&
761-
nextEventWithTile !== nextEvent &&
769+
nextEventWithTile !== nextEvent?.event &&
762770
isSentState(nextEventWithTile.getAssociatedStatus())
763771
) {
764772
isLastSuccessful = false;
@@ -1059,7 +1067,7 @@ interface EventAndShouldShow {
10591067
}
10601068

10611069
abstract class BaseGrouper {
1062-
public static canStartGroup = (panel: MessagePanel, ev: MatrixEvent): boolean => true;
1070+
public static canStartGroup = (_panel: MessagePanel, _ev: EventAndShouldShow): boolean => true;
10631071

10641072
public events: MatrixEvent[] = [];
10651073
// events that we include in the group but then eject out and place above the group.
@@ -1068,17 +1076,20 @@ abstract class BaseGrouper {
10681076

10691077
public constructor(
10701078
public readonly panel: MessagePanel,
1071-
public readonly event: MatrixEvent,
1079+
public readonly firstEventAndShouldShow: EventAndShouldShow,
10721080
public readonly prevEvent: MatrixEvent | null,
1073-
public readonly lastShownEvent: MatrixEvent,
1074-
public readonly nextEvent?: MatrixEvent,
1075-
public readonly nextEventTile?: MatrixEvent,
1081+
public readonly lastShownEvent: MatrixEvent | undefined,
1082+
public readonly nextEvent: EventAndShouldShow | null,
1083+
public readonly nextEventTile?: MatrixEvent | null,
10761084
) {
1077-
this.readMarker = panel.readMarkerForEvent(event.getId(), event === lastShownEvent);
1085+
this.readMarker = panel.readMarkerForEvent(
1086+
firstEventAndShouldShow.event.getId(),
1087+
firstEventAndShouldShow.event === lastShownEvent,
1088+
);
10781089
}
10791090

1080-
public abstract shouldGroup(ev: MatrixEvent): boolean;
1081-
public abstract add(ev: MatrixEvent): void;
1091+
public abstract shouldGroup(ev: EventAndShouldShow): boolean;
1092+
public abstract add(ev: EventAndShouldShow): void;
10821093
public abstract getTiles(): ReactNode[];
10831094
public abstract getNewPrevEvent(): MatrixEvent;
10841095
}
@@ -1099,28 +1110,27 @@ abstract class BaseGrouper {
10991110
// Grouping only events sent by the same user that sent the `m.room.create` and only until
11001111
// the first non-state event, beacon_info event or membership event which is not regarding the sender of the `m.room.create` event
11011112
class CreationGrouper extends BaseGrouper {
1102-
public static canStartGroup = function (panel: MessagePanel, ev: MatrixEvent): boolean {
1103-
return ev.getType() === EventType.RoomCreate;
1113+
public static canStartGroup = function (_panel: MessagePanel, { event }: EventAndShouldShow): boolean {
1114+
return event.getType() === EventType.RoomCreate;
11041115
};
11051116

1106-
public shouldGroup(ev: MatrixEvent): boolean {
1117+
public shouldGroup({ event, shouldShow }: EventAndShouldShow): boolean {
11071118
const panel = this.panel;
1108-
const createEvent = this.event;
1109-
if (!panel.shouldShowEvent(ev)) {
1119+
const createEvent = this.firstEventAndShouldShow.event;
1120+
if (!shouldShow) {
11101121
return true;
11111122
}
1112-
if (panel.wantsDateSeparator(this.event, ev.getDate())) {
1123+
if (panel.wantsDateSeparator(this.firstEventAndShouldShow.event, event.getDate())) {
11131124
return false;
11141125
}
1126+
const eventType = event.getType();
11151127
if (
1116-
ev.getType() === EventType.RoomMember &&
1117-
(ev.getStateKey() !== createEvent.getSender() || ev.getContent()["membership"] !== "join")
1128+
eventType === EventType.RoomMember &&
1129+
(event.getStateKey() !== createEvent.getSender() || event.getContent()["membership"] !== "join")
11181130
) {
11191131
return false;
11201132
}
11211133

1122-
const eventType = ev.getType();
1123-
11241134
// beacons are not part of room creation configuration
11251135
// should be shown in timeline
11261136
if (M_BEACON_INFO.matches(eventType)) {
@@ -1132,17 +1142,17 @@ class CreationGrouper extends BaseGrouper {
11321142
return false;
11331143
}
11341144

1135-
if (ev.isState() && ev.getSender() === createEvent.getSender()) {
1145+
if (event.isState() && event.getSender() === createEvent.getSender()) {
11361146
return true;
11371147
}
11381148

11391149
return false;
11401150
}
11411151

1142-
public add(ev: MatrixEvent): void {
1152+
public add({ event: ev, shouldShow }: EventAndShouldShow): void {
11431153
const panel = this.panel;
11441154
this.readMarker = this.readMarker || panel.readMarkerForEvent(ev.getId(), ev === this.lastShownEvent);
1145-
if (!panel.shouldShowEvent(ev)) {
1155+
if (!shouldShow) {
11461156
return;
11471157
}
11481158
if (ev.getType() === EventType.RoomEncryption) {
@@ -1161,26 +1171,28 @@ class CreationGrouper extends BaseGrouper {
11611171
const panel = this.panel;
11621172
const ret: ReactNode[] = [];
11631173
const isGrouped = true;
1164-
const createEvent = this.event;
1174+
const createEvent = this.firstEventAndShouldShow;
11651175
const lastShownEvent = this.lastShownEvent;
11661176

1167-
if (panel.wantsDateSeparator(this.prevEvent, createEvent.getDate())) {
1168-
const ts = createEvent.getTs();
1177+
if (panel.wantsDateSeparator(this.prevEvent, createEvent.event.getDate())) {
1178+
const ts = createEvent.event.getTs();
11691179
ret.push(
11701180
<li key={ts + "~"}>
1171-
<DateSeparator roomId={createEvent.getRoomId()} ts={ts} />
1181+
<DateSeparator roomId={createEvent.event.getRoomId()} ts={ts} />
11721182
</li>,
11731183
);
11741184
}
11751185

11761186
// If this m.room.create event should be shown (room upgrade) then show it before the summary
1177-
if (panel.shouldShowEvent(createEvent)) {
1187+
if (createEvent.shouldShow) {
11781188
// pass in the createEvent as prevEvent as well so no extra DateSeparator is rendered
1179-
ret.push(...panel.getTilesForEvent(createEvent, createEvent));
1189+
ret.push(...panel.getTilesForEvent(createEvent.event, createEvent.event));
11801190
}
11811191

11821192
for (const ejected of this.ejectedEvents) {
1183-
ret.push(...panel.getTilesForEvent(createEvent, ejected, createEvent === lastShownEvent, isGrouped));
1193+
ret.push(
1194+
...panel.getTilesForEvent(createEvent.event, ejected, createEvent.event === lastShownEvent, isGrouped),
1195+
);
11841196
}
11851197

11861198
const eventTiles = this.events
@@ -1227,14 +1239,17 @@ class CreationGrouper extends BaseGrouper {
12271239
}
12281240

12291241
public getNewPrevEvent(): MatrixEvent {
1230-
return this.event;
1242+
return this.firstEventAndShouldShow.event;
12311243
}
12321244
}
12331245

12341246
// Wrap consecutive grouped events in a ListSummary
12351247
class MainGrouper extends BaseGrouper {
1236-
public static canStartGroup = function (panel: MessagePanel, ev: MatrixEvent): boolean {
1237-
if (!panel.shouldShowEvent(ev)) return false;
1248+
public static canStartGroup = function (
1249+
panel: MessagePanel,
1250+
{ event: ev, shouldShow }: EventAndShouldShow,
1251+
): boolean {
1252+
if (!shouldShow) return false;
12381253

12391254
if (ev.isState() && groupedStateEvents.includes(ev.getType() as EventType)) {
12401255
return true;
@@ -1253,18 +1268,18 @@ class MainGrouper extends BaseGrouper {
12531268

12541269
public constructor(
12551270
public readonly panel: MessagePanel,
1256-
public readonly event: MatrixEvent,
1271+
public readonly firstEventAndShouldShow: EventAndShouldShow,
12571272
public readonly prevEvent: MatrixEvent | null,
1258-
public readonly lastShownEvent: MatrixEvent,
1259-
nextEvent: MatrixEvent,
1260-
nextEventTile: MatrixEvent,
1273+
public readonly lastShownEvent: MatrixEvent | undefined,
1274+
nextEvent: EventAndShouldShow | null,
1275+
nextEventTile: MatrixEvent | null,
12611276
) {
1262-
super(panel, event, prevEvent, lastShownEvent, nextEvent, nextEventTile);
1263-
this.events = [event];
1277+
super(panel, firstEventAndShouldShow, prevEvent, lastShownEvent, nextEvent, nextEventTile);
1278+
this.events = [firstEventAndShouldShow.event];
12641279
}
12651280

1266-
public shouldGroup(ev: MatrixEvent): boolean {
1267-
if (!this.panel.shouldShowEvent(ev)) {
1281+
public shouldGroup({ event: ev, shouldShow }: EventAndShouldShow): boolean {
1282+
if (!shouldShow) {
12681283
// absorb hidden events so that they do not break up streams of messages & redaction events being grouped
12691284
return true;
12701285
}
@@ -1283,13 +1298,13 @@ class MainGrouper extends BaseGrouper {
12831298
return false;
12841299
}
12851300

1286-
public add(ev: MatrixEvent): void {
1301+
public add({ event: ev, shouldShow }: EventAndShouldShow): void {
12871302
if (ev.getType() === EventType.RoomMember) {
12881303
// We can ignore any events that don't actually have a message to display
12891304
if (!hasText(ev, this.panel.showHiddenEvents)) return;
12901305
}
12911306
this.readMarker = this.readMarker || this.panel.readMarkerForEvent(ev.getId(), ev === this.lastShownEvent);
1292-
if (!this.panel.showHiddenEvents && !this.panel.shouldShowEvent(ev)) {
1307+
if (!this.panel.showHiddenEvents && !shouldShow) {
12931308
// absorb hidden events to not split the summary
12941309
return;
12951310
}
@@ -1393,6 +1408,10 @@ const groupers = [CreationGrouper, MainGrouper];
13931408
* event that is >start items through the list, and is shown.
13941409
*/
13951410
function findFirstShownAfter(start: number, events: EventAndShouldShow[]): MatrixEvent | null {
1411+
// Note: this could be done with something like:
1412+
// events.slice(i + 1).find((e) => e.shouldShow)?.event ?? null;
1413+
// but it is ~10% slower, and this is on the critical path.
1414+
13961415
for (let n = start + 1; n < events.length; n++) {
13971416
const { event, shouldShow } = events[n];
13981417
if (shouldShow) {

0 commit comments

Comments
 (0)