Skip to content
This repository was archived by the owner on Sep 11, 2024. It is now read-only.

Commit 102c4e5

Browse files
authored
Improve performance of rendering a room with many hidden events (#10131)
* Test MessagePanel rendering with hidden events * Cache the results of shouldShowEvent in MessagePanel * Combine an event and its shouldShow info into a single object
1 parent 580857e commit 102c4e5

File tree

3 files changed

+98
-17
lines changed

3 files changed

+98
-17
lines changed

src/components/structures/MessagePanel.tsx

+56-17
Original file line numberDiff line numberDiff line change
@@ -561,14 +561,29 @@ export default class MessagePanel extends React.Component<IProps, IState> {
561561
});
562562
};
563563

564-
private getNextEventInfo(arr: MatrixEvent[], i: number): { nextEvent: MatrixEvent; nextTile: MatrixEvent } {
565-
const nextEvent = i < arr.length - 1 ? arr[i + 1] : null;
564+
/**
565+
* Find the next event in the list, and the next visible event in the list.
566+
*
567+
* @param event - the list of events to look in and whether they are shown
568+
* @param i - where in the list we are now
569+
*
570+
* @returns { nextEvent, nextTile }
571+
*
572+
* nextEvent is the event after i in the supplied array.
573+
*
574+
* nextTile is the first event in the array after i that we will show a tile
575+
* for. It is used to to determine the 'last successful' flag when rendering
576+
* the tile.
577+
*/
578+
private getNextEventInfo(
579+
events: EventAndShouldShow[],
580+
i: number,
581+
): { nextEvent: MatrixEvent | null; nextTile: MatrixEvent | null } {
582+
// WARNING: this method is on a hot path.
583+
584+
const nextEvent = i < events.length - 1 ? events[i + 1].event : null;
566585

567-
// The next event with tile is used to to determine the 'last successful' flag
568-
// when rendering the tile. The shouldShowEvent function is pretty quick at what
569-
// it does, so this should have no significant cost even when a room is used for
570-
// not-chat purposes.
571-
const nextTile = arr.slice(i + 1).find((e) => this.shouldShowEvent(e));
586+
const nextTile = findFirstShownAfter(i, events);
572587

573588
return { nextEvent, nextTile };
574589
}
@@ -587,20 +602,21 @@ export default class MessagePanel extends React.Component<IProps, IState> {
587602
}
588603

589604
private getEventTiles(): ReactNode[] {
590-
let i;
591-
592605
// first figure out which is the last event in the list which we're
593606
// actually going to show; this allows us to behave slightly
594607
// differently for the last event in the list. (eg show timestamp)
595608
//
596609
// we also need to figure out which is the last event we show which isn't
597610
// a local echo, to manage the read-marker.
598-
let lastShownEvent;
611+
let lastShownEvent: MatrixEvent | undefined;
612+
const events: EventAndShouldShow[] = this.props.events.map((event) => {
613+
return { event, shouldShow: this.shouldShowEvent(event) };
614+
});
599615

600616
let lastShownNonLocalEchoIndex = -1;
601-
for (i = this.props.events.length - 1; i >= 0; i--) {
602-
const mxEv = this.props.events[i];
603-
if (!this.shouldShowEvent(mxEv)) {
617+
for (let i = events.length - 1; i >= 0; i--) {
618+
const { event: mxEv, shouldShow } = events[i];
619+
if (!shouldShow) {
604620
continue;
605621
}
606622

@@ -631,11 +647,11 @@ export default class MessagePanel extends React.Component<IProps, IState> {
631647

632648
let grouper: BaseGrouper = null;
633649

634-
for (i = 0; i < this.props.events.length; i++) {
635-
const mxEv = this.props.events[i];
650+
for (let i = 0; i < events.length; i++) {
651+
const { event: mxEv, shouldShow } = events[i];
636652
const eventId = mxEv.getId();
637653
const last = mxEv === lastShownEvent;
638-
const { nextEvent, nextTile } = this.getNextEventInfo(this.props.events, i);
654+
const { nextEvent, nextTile } = this.getNextEventInfo(events, i);
639655

640656
if (grouper) {
641657
if (grouper.shouldGroup(mxEv)) {
@@ -658,7 +674,7 @@ export default class MessagePanel extends React.Component<IProps, IState> {
658674
}
659675

660676
if (!grouper) {
661-
if (this.shouldShowEvent(mxEv)) {
677+
if (shouldShow) {
662678
// make sure we unpack the array returned by getTilesForEvent,
663679
// otherwise React will auto-generate keys, and we will end up
664680
// replacing all the DOM elements every time we paginate.
@@ -1040,6 +1056,15 @@ export default class MessagePanel extends React.Component<IProps, IState> {
10401056
}
10411057
}
10421058

1059+
/**
1060+
* Holds on to an event, caching the information about whether it should be
1061+
* shown. Avoids calling shouldShowEvent more times than we need to.
1062+
*/
1063+
interface EventAndShouldShow {
1064+
event: MatrixEvent;
1065+
shouldShow: boolean;
1066+
}
1067+
10431068
abstract class BaseGrouper {
10441069
public static canStartGroup = (panel: MessagePanel, ev: MatrixEvent): boolean => true;
10451070

@@ -1369,3 +1394,17 @@ class MainGrouper extends BaseGrouper {
13691394

13701395
// all the grouper classes that we use, ordered by priority
13711396
const groupers = [CreationGrouper, MainGrouper];
1397+
1398+
/**
1399+
* Look through the supplied list of EventAndShouldShow, and return the first
1400+
* event that is >start items through the list, and is shown.
1401+
*/
1402+
function findFirstShownAfter(start: number, events: EventAndShouldShow[]): MatrixEvent | null {
1403+
for (let n = start + 1; n < events.length; n++) {
1404+
const { event, shouldShow } = events[n];
1405+
if (shouldShow) {
1406+
return event;
1407+
}
1408+
}
1409+
return null;
1410+
}

test/components/structures/MessagePanel-test.tsx

+21
Original file line numberDiff line numberDiff line change
@@ -669,6 +669,27 @@ describe("MessagePanel", function () {
669669
expect(els.length).toEqual(1);
670670
expect(els[0].getAttribute("data-scroll-tokens")?.split(",")).toHaveLength(3);
671671
});
672+
673+
it("should handle large numbers of hidden events quickly", () => {
674+
// Increase the length of the loop here to test performance issues with
675+
// rendering
676+
677+
const events = [];
678+
for (let i = 0; i < 100; i++) {
679+
events.push(
680+
TestUtilsMatrix.mkEvent({
681+
event: true,
682+
type: "unknown.event.type",
683+
content: { key: "value" },
684+
room: "!room:id",
685+
user: "@user:id",
686+
ts: 1000000 + i,
687+
}),
688+
);
689+
}
690+
const { asFragment } = render(getComponent({ events }, { showHiddenEvents: false }));
691+
expect(asFragment()).toMatchSnapshot();
692+
});
672693
});
673694

674695
describe("shouldFormContinuation", () => {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
2+
3+
exports[`MessagePanel should handle large numbers of hidden events quickly 1`] = `
4+
<DocumentFragment>
5+
<div
6+
class="mx_AutoHideScrollbar mx_ScrollPanel cls"
7+
tabindex="-1"
8+
>
9+
<div
10+
class="mx_RoomView_messageListWrapper"
11+
>
12+
<ol
13+
aria-live="polite"
14+
class="mx_RoomView_MessageList"
15+
style="height: 400px;"
16+
/>
17+
</div>
18+
</div>
19+
);
20+
</DocumentFragment>
21+
`;

0 commit comments

Comments
 (0)