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

Improve performance of rendering a room with many hidden events #10131

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 56 additions & 17 deletions src/components/structures/MessagePanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -561,14 +561,29 @@ export default class MessagePanel extends React.Component<IProps, IState> {
});
};

private getNextEventInfo(arr: MatrixEvent[], i: number): { nextEvent: MatrixEvent; nextTile: MatrixEvent } {
const nextEvent = i < arr.length - 1 ? arr[i + 1] : null;
/**
* Find the next event in the list, and the next visible event in the list.
*
* @param event - the list of events to look in and whether they are shown
* @param i - where in the list we are now
*
* @returns { nextEvent, nextTile }
*
* nextEvent is the event after i in the supplied array.
*
* nextTile is the first event in the array after i that we will show a tile
* for. It is used to to determine the 'last successful' flag when rendering
* the tile.
*/
private getNextEventInfo(
events: EventAndShouldShow[],
i: number,
): { nextEvent: MatrixEvent | null; nextTile: MatrixEvent | null } {
// WARNING: this method is on a hot path.

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

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

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

private getEventTiles(): ReactNode[] {
let i;

// first figure out which is the last event in the list which we're
// actually going to show; this allows us to behave slightly
// differently for the last event in the list. (eg show timestamp)
//
// we also need to figure out which is the last event we show which isn't
// a local echo, to manage the read-marker.
let lastShownEvent;
let lastShownEvent: MatrixEvent | undefined;
const events: EventAndShouldShow[] = this.props.events.map((event) => {
return { event, shouldShow: this.shouldShowEvent(event) };
});

let lastShownNonLocalEchoIndex = -1;
for (i = this.props.events.length - 1; i >= 0; i--) {
const mxEv = this.props.events[i];
if (!this.shouldShowEvent(mxEv)) {
for (let i = events.length - 1; i >= 0; i--) {
const { event: mxEv, shouldShow } = events[i];
if (!shouldShow) {
continue;
}

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

let grouper: BaseGrouper = null;

for (i = 0; i < this.props.events.length; i++) {
const mxEv = this.props.events[i];
for (let i = 0; i < events.length; i++) {
const { event: mxEv, shouldShow } = events[i];
const eventId = mxEv.getId();
const last = mxEv === lastShownEvent;
const { nextEvent, nextTile } = this.getNextEventInfo(this.props.events, i);
const { nextEvent, nextTile } = this.getNextEventInfo(events, i);

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

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

/**
* Holds on to an event, caching the information about whether it should be
* shown. Avoids calling shouldShowEvent more times than we need to.
*/
interface EventAndShouldShow {
event: MatrixEvent;
shouldShow: boolean;
}

abstract class BaseGrouper {
public static canStartGroup = (panel: MessagePanel, ev: MatrixEvent): boolean => true;

Expand Down Expand Up @@ -1369,3 +1394,17 @@ class MainGrouper extends BaseGrouper {

// all the grouper classes that we use, ordered by priority
const groupers = [CreationGrouper, MainGrouper];

/**
* Look through the supplied list of EventAndShouldShow, and return the first
* event that is >start items through the list, and is shown.
*/
function findFirstShownAfter(start: number, events: EventAndShouldShow[]): MatrixEvent | null {
for (let n = start + 1; n < events.length; n++) {
const { event, shouldShow } = events[n];
if (shouldShow) {
return event;
}
}
return null;
}
21 changes: 21 additions & 0 deletions test/components/structures/MessagePanel-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -669,6 +669,27 @@ describe("MessagePanel", function () {
expect(els.length).toEqual(1);
expect(els[0].getAttribute("data-scroll-tokens")?.split(",")).toHaveLength(3);
});

it("should handle large numbers of hidden events quickly", () => {
// Increase the length of the loop here to test performance issues with
// rendering

const events = [];
for (let i = 0; i < 100; i++) {
events.push(
TestUtilsMatrix.mkEvent({
event: true,
type: "unknown.event.type",
content: { key: "value" },
room: "!room:id",
user: "@user:id",
ts: 1000000 + i,
}),
);
}
const { asFragment } = render(getComponent({ events }, { showHiddenEvents: false }));
expect(asFragment()).toMatchSnapshot();
});
});

describe("shouldFormContinuation", () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`MessagePanel should handle large numbers of hidden events quickly 1`] = `
<DocumentFragment>
<div
class="mx_AutoHideScrollbar mx_ScrollPanel cls"
tabindex="-1"
>
<div
class="mx_RoomView_messageListWrapper"
>
<ol
aria-live="polite"
class="mx_RoomView_MessageList"
style="height: 400px;"
/>
</div>
</div>
);
</DocumentFragment>
`;