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
This commit is contained in:
Andy Balaam 2023-03-20 09:50:07 +00:00 committed by GitHub
parent bc60e59eda
commit ca0dfb6c1e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 266 additions and 68 deletions

View file

@ -564,7 +564,7 @@ export default class MessagePanel extends React.Component<IProps, IState> {
/** /**
* Find the next event in the list, and the next visible event in the list. * 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 events - the list of events to look in and whether they are shown
* @param i - where in the list we are now * @param i - where in the list we are now
* *
* @returns { nextEvent, nextTile } * @returns { nextEvent, nextTile }
@ -578,14 +578,14 @@ export default class MessagePanel extends React.Component<IProps, IState> {
private getNextEventInfo( private getNextEventInfo(
events: EventAndShouldShow[], events: EventAndShouldShow[],
i: number, i: number,
): { nextEvent: MatrixEvent | null; nextTile: MatrixEvent | null } { ): { nextEventAndShouldShow: EventAndShouldShow | null; nextTile: MatrixEvent | null } {
// WARNING: this method is on a hot path. // WARNING: this method is on a hot path.
const nextEvent = i < events.length - 1 ? events[i + 1].event : null; const nextEventAndShouldShow = i < events.length - 1 ? events[i + 1] : null;
const nextTile = findFirstShownAfter(i, events); const nextTile = findFirstShownAfter(i, events);
return { nextEvent, nextTile }; return { nextEventAndShouldShow, nextTile };
} }
private get pendingEditItem(): string | undefined { private get pendingEditItem(): string | undefined {
@ -615,16 +615,16 @@ export default class MessagePanel extends React.Component<IProps, IState> {
let lastShownNonLocalEchoIndex = -1; let lastShownNonLocalEchoIndex = -1;
for (let i = events.length - 1; i >= 0; i--) { for (let i = events.length - 1; i >= 0; i--) {
const { event: mxEv, shouldShow } = events[i]; const { event, shouldShow } = events[i];
if (!shouldShow) { if (!shouldShow) {
continue; continue;
} }
if (lastShownEvent === undefined) { if (lastShownEvent === undefined) {
lastShownEvent = mxEv; lastShownEvent = event;
} }
if (mxEv.status) { if (event.status) {
// this is a local echo // this is a local echo
continue; continue;
} }
@ -641,20 +641,21 @@ export default class MessagePanel extends React.Component<IProps, IState> {
// to assume that sent receipts are to be shown more often. // to assume that sent receipts are to be shown more often.
this.readReceiptsByEvent = {}; this.readReceiptsByEvent = {};
if (this.props.showReadReceipts) { if (this.props.showReadReceipts) {
this.readReceiptsByEvent = this.getReadReceiptsByShownEvent(); this.readReceiptsByEvent = this.getReadReceiptsByShownEvent(events);
} }
let grouper: BaseGrouper | null = null; let grouper: BaseGrouper | null = null;
for (let i = 0; i < events.length; i++) { for (let i = 0; i < events.length; i++) {
const { event: mxEv, shouldShow } = events[i]; const eventAndShouldShow = events[i];
const eventId = mxEv.getId(); const { event, shouldShow } = eventAndShouldShow;
const last = mxEv === lastShownEvent; const eventId = event.getId();
const { nextEvent, nextTile } = this.getNextEventInfo(events, i); const last = event === lastShownEvent;
const { nextEventAndShouldShow, nextTile } = this.getNextEventInfo(events, i);
if (grouper) { if (grouper) {
if (grouper.shouldGroup(mxEv)) { if (grouper.shouldGroup(eventAndShouldShow)) {
grouper.add(mxEv); grouper.add(eventAndShouldShow);
continue; continue;
} else { } else {
// not part of group, so get the group tiles, close the // not part of group, so get the group tiles, close the
@ -666,8 +667,15 @@ export default class MessagePanel extends React.Component<IProps, IState> {
} }
for (const Grouper of groupers) { for (const Grouper of groupers) {
if (Grouper.canStartGroup(this, mxEv) && !this.props.disableGrouping) { if (Grouper.canStartGroup(this, eventAndShouldShow) && !this.props.disableGrouping) {
grouper = new Grouper(this, mxEv, prevEvent, lastShownEvent, nextEvent, nextTile); grouper = new Grouper(
this,
eventAndShouldShow,
prevEvent,
lastShownEvent,
nextEventAndShouldShow,
nextTile,
);
break; // break on first grouper break; // break on first grouper
} }
} }
@ -677,8 +685,8 @@ export default class MessagePanel extends React.Component<IProps, IState> {
// make sure we unpack the array returned by getTilesForEvent, // make sure we unpack the array returned by getTilesForEvent,
// otherwise React will auto-generate keys, and we will end up // otherwise React will auto-generate keys, and we will end up
// replacing all the DOM elements every time we paginate. // replacing all the DOM elements every time we paginate.
ret.push(...this.getTilesForEvent(prevEvent, mxEv, last, false, nextEvent, nextTile)); ret.push(...this.getTilesForEvent(prevEvent, event, last, false, nextEventAndShouldShow, nextTile));
prevEvent = mxEv; prevEvent = event;
} }
const readMarker = this.readMarkerForEvent(eventId, i >= lastShownNonLocalEchoIndex); const readMarker = this.readMarkerForEvent(eventId, i >= lastShownNonLocalEchoIndex);
@ -698,8 +706,8 @@ export default class MessagePanel extends React.Component<IProps, IState> {
mxEv: MatrixEvent, mxEv: MatrixEvent,
last = false, last = false,
isGrouped = false, isGrouped = false,
nextEvent?: MatrixEvent, nextEvent: EventAndShouldShow | null = null,
nextEventWithTile?: MatrixEvent, nextEventWithTile: MatrixEvent | null = null,
): ReactNode[] { ): ReactNode[] {
const ret: ReactNode[] = []; const ret: ReactNode[] = [];
@ -745,12 +753,12 @@ export default class MessagePanel extends React.Component<IProps, IState> {
const readReceipts = this.readReceiptsByEvent[eventId]; const readReceipts = this.readReceiptsByEvent[eventId];
let isLastSuccessful = false; let isLastSuccessful = false;
const isSentState = (s: EventStatus): boolean => !s || s === EventStatus.SENT; const isSentState = (s: EventStatus | null): boolean => !s || s === EventStatus.SENT;
const isSent = isSentState(mxEv.getAssociatedStatus()); const isSent = isSentState(mxEv.getAssociatedStatus());
const hasNextEvent = nextEvent && this.shouldShowEvent(nextEvent); const hasNextEvent = nextEvent?.shouldShow;
if (!hasNextEvent && isSent) { if (!hasNextEvent && isSent) {
isLastSuccessful = true; isLastSuccessful = true;
} else if (hasNextEvent && isSent && !isSentState(nextEvent.getAssociatedStatus())) { } else if (hasNextEvent && isSent && !isSentState(nextEvent.event.getAssociatedStatus())) {
isLastSuccessful = true; isLastSuccessful = true;
} }
@ -758,7 +766,7 @@ export default class MessagePanel extends React.Component<IProps, IState> {
// hidden then we're not the last successful. // hidden then we're not the last successful.
if ( if (
nextEventWithTile && nextEventWithTile &&
nextEventWithTile !== nextEvent && nextEventWithTile !== nextEvent?.event &&
isSentState(nextEventWithTile.getAssociatedStatus()) isSentState(nextEventWithTile.getAssociatedStatus())
) { ) {
isLastSuccessful = false; isLastSuccessful = false;
@ -861,7 +869,7 @@ export default class MessagePanel extends React.Component<IProps, IState> {
// Get an object that maps from event ID to a list of read receipts that // Get an object that maps from event ID to a list of read receipts that
// should be shown next to that event. If a hidden event has read receipts, // should be shown next to that event. If a hidden event has read receipts,
// they are folded into the receipts of the last shown event. // they are folded into the receipts of the last shown event.
private getReadReceiptsByShownEvent(): Record<string, IReadReceiptProps[]> { private getReadReceiptsByShownEvent(events: EventAndShouldShow[]): Record<string, IReadReceiptProps[]> {
const receiptsByEvent: Record<string, IReadReceiptProps[]> = {}; const receiptsByEvent: Record<string, IReadReceiptProps[]> = {};
const receiptsByUserId: Record< const receiptsByUserId: Record<
string, string,
@ -872,8 +880,8 @@ export default class MessagePanel extends React.Component<IProps, IState> {
> = {}; > = {};
let lastShownEventId; let lastShownEventId;
for (const event of this.props.events) { for (const { event, shouldShow } of events) {
if (this.shouldShowEvent(event)) { if (shouldShow) {
lastShownEventId = event.getId(); lastShownEventId = event.getId();
} }
if (!lastShownEventId) { if (!lastShownEventId) {
@ -1065,7 +1073,7 @@ interface EventAndShouldShow {
} }
abstract class BaseGrouper { abstract class BaseGrouper {
public static canStartGroup = (panel: MessagePanel, ev: MatrixEvent): boolean => true; public static canStartGroup = (_panel: MessagePanel, _ev: EventAndShouldShow): boolean => true;
public events: MatrixEvent[] = []; public events: MatrixEvent[] = [];
// events that we include in the group but then eject out and place above the group. // events that we include in the group but then eject out and place above the group.
@ -1074,17 +1082,20 @@ abstract class BaseGrouper {
public constructor( public constructor(
public readonly panel: MessagePanel, public readonly panel: MessagePanel,
public readonly event: MatrixEvent, public readonly firstEventAndShouldShow: EventAndShouldShow,
public readonly prevEvent: MatrixEvent | null, public readonly prevEvent: MatrixEvent | null,
public readonly lastShownEvent: MatrixEvent, public readonly lastShownEvent: MatrixEvent | undefined,
public readonly nextEvent?: MatrixEvent, public readonly nextEvent: EventAndShouldShow | null,
public readonly nextEventTile?: MatrixEvent, public readonly nextEventTile?: MatrixEvent | null,
) { ) {
this.readMarker = panel.readMarkerForEvent(event.getId(), event === lastShownEvent); this.readMarker = panel.readMarkerForEvent(
firstEventAndShouldShow.event.getId(),
firstEventAndShouldShow.event === lastShownEvent,
);
} }
public abstract shouldGroup(ev: MatrixEvent): boolean; public abstract shouldGroup(ev: EventAndShouldShow): boolean;
public abstract add(ev: MatrixEvent): void; public abstract add(ev: EventAndShouldShow): void;
public abstract getTiles(): ReactNode[]; public abstract getTiles(): ReactNode[];
public abstract getNewPrevEvent(): MatrixEvent; public abstract getNewPrevEvent(): MatrixEvent;
} }
@ -1105,28 +1116,27 @@ abstract class BaseGrouper {
// Grouping only events sent by the same user that sent the `m.room.create` and only until // Grouping only events sent by the same user that sent the `m.room.create` and only until
// the first non-state event, beacon_info event or membership event which is not regarding the sender of the `m.room.create` event // the first non-state event, beacon_info event or membership event which is not regarding the sender of the `m.room.create` event
class CreationGrouper extends BaseGrouper { class CreationGrouper extends BaseGrouper {
public static canStartGroup = function (panel: MessagePanel, ev: MatrixEvent): boolean { public static canStartGroup = function (_panel: MessagePanel, { event }: EventAndShouldShow): boolean {
return ev.getType() === EventType.RoomCreate; return event.getType() === EventType.RoomCreate;
}; };
public shouldGroup(ev: MatrixEvent): boolean { public shouldGroup({ event, shouldShow }: EventAndShouldShow): boolean {
const panel = this.panel; const panel = this.panel;
const createEvent = this.event; const createEvent = this.firstEventAndShouldShow.event;
if (!panel.shouldShowEvent(ev)) { if (!shouldShow) {
return true; return true;
} }
if (panel.wantsDateSeparator(this.event, ev.getDate())) { if (panel.wantsDateSeparator(this.firstEventAndShouldShow.event, event.getDate())) {
return false; return false;
} }
const eventType = event.getType();
if ( if (
ev.getType() === EventType.RoomMember && eventType === EventType.RoomMember &&
(ev.getStateKey() !== createEvent.getSender() || ev.getContent()["membership"] !== "join") (event.getStateKey() !== createEvent.getSender() || event.getContent()["membership"] !== "join")
) { ) {
return false; return false;
} }
const eventType = ev.getType();
// beacons are not part of room creation configuration // beacons are not part of room creation configuration
// should be shown in timeline // should be shown in timeline
if (M_BEACON_INFO.matches(eventType)) { if (M_BEACON_INFO.matches(eventType)) {
@ -1138,17 +1148,17 @@ class CreationGrouper extends BaseGrouper {
return false; return false;
} }
if (ev.isState() && ev.getSender() === createEvent.getSender()) { if (event.isState() && event.getSender() === createEvent.getSender()) {
return true; return true;
} }
return false; return false;
} }
public add(ev: MatrixEvent): void { public add({ event: ev, shouldShow }: EventAndShouldShow): void {
const panel = this.panel; const panel = this.panel;
this.readMarker = this.readMarker || panel.readMarkerForEvent(ev.getId(), ev === this.lastShownEvent); this.readMarker = this.readMarker || panel.readMarkerForEvent(ev.getId(), ev === this.lastShownEvent);
if (!panel.shouldShowEvent(ev)) { if (!shouldShow) {
return; return;
} }
if (ev.getType() === EventType.RoomEncryption) { if (ev.getType() === EventType.RoomEncryption) {
@ -1167,26 +1177,28 @@ class CreationGrouper extends BaseGrouper {
const panel = this.panel; const panel = this.panel;
const ret: ReactNode[] = []; const ret: ReactNode[] = [];
const isGrouped = true; const isGrouped = true;
const createEvent = this.event; const createEvent = this.firstEventAndShouldShow;
const lastShownEvent = this.lastShownEvent; const lastShownEvent = this.lastShownEvent;
if (panel.wantsDateSeparator(this.prevEvent, createEvent.getDate())) { if (panel.wantsDateSeparator(this.prevEvent, createEvent.event.getDate())) {
const ts = createEvent.getTs(); const ts = createEvent.event.getTs();
ret.push( ret.push(
<li key={ts + "~"}> <li key={ts + "~"}>
<DateSeparator roomId={createEvent.getRoomId()} ts={ts} /> <DateSeparator roomId={createEvent.event.getRoomId()} ts={ts} />
</li>, </li>,
); );
} }
// If this m.room.create event should be shown (room upgrade) then show it before the summary // If this m.room.create event should be shown (room upgrade) then show it before the summary
if (panel.shouldShowEvent(createEvent)) { if (createEvent.shouldShow) {
// pass in the createEvent as prevEvent as well so no extra DateSeparator is rendered // pass in the createEvent as prevEvent as well so no extra DateSeparator is rendered
ret.push(...panel.getTilesForEvent(createEvent, createEvent)); ret.push(...panel.getTilesForEvent(createEvent.event, createEvent.event));
} }
for (const ejected of this.ejectedEvents) { for (const ejected of this.ejectedEvents) {
ret.push(...panel.getTilesForEvent(createEvent, ejected, createEvent === lastShownEvent, isGrouped)); ret.push(
...panel.getTilesForEvent(createEvent.event, ejected, createEvent.event === lastShownEvent, isGrouped),
);
} }
const eventTiles = this.events const eventTiles = this.events
@ -1233,14 +1245,17 @@ class CreationGrouper extends BaseGrouper {
} }
public getNewPrevEvent(): MatrixEvent { public getNewPrevEvent(): MatrixEvent {
return this.event; return this.firstEventAndShouldShow.event;
} }
} }
// Wrap consecutive grouped events in a ListSummary // Wrap consecutive grouped events in a ListSummary
class MainGrouper extends BaseGrouper { class MainGrouper extends BaseGrouper {
public static canStartGroup = function (panel: MessagePanel, ev: MatrixEvent): boolean { public static canStartGroup = function (
if (!panel.shouldShowEvent(ev)) return false; panel: MessagePanel,
{ event: ev, shouldShow }: EventAndShouldShow,
): boolean {
if (!shouldShow) return false;
if (ev.isState() && groupedStateEvents.includes(ev.getType() as EventType)) { if (ev.isState() && groupedStateEvents.includes(ev.getType() as EventType)) {
return true; return true;
@ -1259,18 +1274,18 @@ class MainGrouper extends BaseGrouper {
public constructor( public constructor(
public readonly panel: MessagePanel, public readonly panel: MessagePanel,
public readonly event: MatrixEvent, public readonly firstEventAndShouldShow: EventAndShouldShow,
public readonly prevEvent: MatrixEvent | null, public readonly prevEvent: MatrixEvent | null,
public readonly lastShownEvent: MatrixEvent, public readonly lastShownEvent: MatrixEvent | undefined,
nextEvent: MatrixEvent, nextEvent: EventAndShouldShow | null,
nextEventTile: MatrixEvent, nextEventTile: MatrixEvent | null,
) { ) {
super(panel, event, prevEvent, lastShownEvent, nextEvent, nextEventTile); super(panel, firstEventAndShouldShow, prevEvent, lastShownEvent, nextEvent, nextEventTile);
this.events = [event]; this.events = [firstEventAndShouldShow.event];
} }
public shouldGroup(ev: MatrixEvent): boolean { public shouldGroup({ event: ev, shouldShow }: EventAndShouldShow): boolean {
if (!this.panel.shouldShowEvent(ev)) { if (!shouldShow) {
// absorb hidden events so that they do not break up streams of messages & redaction events being grouped // absorb hidden events so that they do not break up streams of messages & redaction events being grouped
return true; return true;
} }
@ -1289,13 +1304,13 @@ class MainGrouper extends BaseGrouper {
return false; return false;
} }
public add(ev: MatrixEvent): void { public add({ event: ev, shouldShow }: EventAndShouldShow): void {
if (ev.getType() === EventType.RoomMember) { if (ev.getType() === EventType.RoomMember) {
// We can ignore any events that don't actually have a message to display // We can ignore any events that don't actually have a message to display
if (!hasText(ev, this.panel.showHiddenEvents)) return; if (!hasText(ev, this.panel.showHiddenEvents)) return;
} }
this.readMarker = this.readMarker || this.panel.readMarkerForEvent(ev.getId(), ev === this.lastShownEvent); this.readMarker = this.readMarker || this.panel.readMarkerForEvent(ev.getId(), ev === this.lastShownEvent);
if (!this.panel.showHiddenEvents && !this.panel.shouldShowEvent(ev)) { if (!this.panel.showHiddenEvents && !shouldShow) {
// absorb hidden events to not split the summary // absorb hidden events to not split the summary
return; return;
} }
@ -1399,6 +1414,10 @@ const groupers = [CreationGrouper, MainGrouper];
* event that is >start items through the list, and is shown. * event that is >start items through the list, and is shown.
*/ */
function findFirstShownAfter(start: number, events: EventAndShouldShow[]): MatrixEvent | null { function findFirstShownAfter(start: number, events: EventAndShouldShow[]): MatrixEvent | null {
// Note: this could be done with something like:
// events.slice(i + 1).find((e) => e.shouldShow)?.event ?? null;
// but it is ~10% slower, and this is on the critical path.
for (let n = start + 1; n < events.length; n++) { for (let n = start + 1; n < events.length; n++) {
const { event, shouldShow } = events[n]; const { event, shouldShow } = events[n];
if (shouldShow) { if (shouldShow) {

View file

@ -690,6 +690,56 @@ describe("MessagePanel", function () {
const { asFragment } = render(getComponent({ events }, { showHiddenEvents: false })); const { asFragment } = render(getComponent({ events }, { showHiddenEvents: false }));
expect(asFragment()).toMatchSnapshot(); expect(asFragment()).toMatchSnapshot();
}); });
it("should handle lots of room creation events quickly", () => {
// Increase the length of the loop here to test performance issues with
// rendering
const events = [TestUtilsMatrix.mkRoomCreateEvent("@user:id", "!room:id")];
for (let i = 0; i < 100; i++) {
events.push(
TestUtilsMatrix.mkMembership({
mship: "join",
prevMship: "join",
room: "!room:id",
user: "@user:id",
event: true,
skey: "123",
}),
);
}
const { asFragment } = render(getComponent({ events }, { showHiddenEvents: false }));
expect(asFragment()).toMatchSnapshot();
});
it("should handle lots of membership 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.mkMembership({
mship: "join",
prevMship: "join",
room: "!room:id",
user: "@user:id",
event: true,
skey: "123",
}),
);
}
const { asFragment } = render(getComponent({ events }, { showHiddenEvents: true }));
const cpt = asFragment();
// Ignore properties that change every time
cpt.querySelectorAll("li").forEach((li) => {
li.setAttribute("data-scroll-tokens", "__scroll_tokens__");
li.setAttribute("data-testid", "__testid__");
});
expect(cpt).toMatchSnapshot();
});
}); });
describe("shouldFormContinuation", () => { describe("shouldFormContinuation", () => {

View file

@ -19,3 +19,132 @@ exports[`MessagePanel should handle large numbers of hidden events quickly 1`] =
); );
</DocumentFragment> </DocumentFragment>
`; `;
exports[`MessagePanel should handle lots of membership 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;"
>
<li
data-scroll-tokens="__scroll_tokens__"
data-testid="__testid__"
>
<div
aria-label="Thu, Jan 1 1970"
class="mx_DateSeparator"
role="separator"
tabindex="-1"
>
<hr
role="none"
/>
<h2
aria-hidden="true"
>
Thu, Jan 1 1970
</h2>
<hr
role="none"
/>
</div>
</li>
<div
class="mx_EventTileBubble mx_HistoryTile"
>
<div
class="mx_EventTileBubble_title"
>
You can't see earlier messages
</div>
</div>
<li
class="mx_GenericEventListSummary"
data-expanded="false"
data-layout="group"
data-scroll-tokens="__scroll_tokens__"
data-testid="__testid__"
>
<div
aria-expanded="false"
class="mx_AccessibleButton mx_GenericEventListSummary_toggle mx_AccessibleButton_hasKind mx_AccessibleButton_kind_link_inline"
role="button"
tabindex="0"
>
expand
</div>
<div
class="mx_EventTile_line"
>
<div
class="mx_EventTile_info"
>
<span
class="mx_GenericEventListSummary_avatars"
>
<span
class="mx_BaseAvatar"
role="presentation"
>
<span
aria-hidden="true"
class="mx_BaseAvatar_initial"
style="font-size: 9.1px; width: 14px; line-height: 14px;"
>
U
</span>
<img
alt=""
aria-hidden="true"
class="mx_BaseAvatar_image"
data-testid="avatar-img"
src="data:image/png;base64,00"
style="width: 14px; height: 14px;"
title="@user:id"
/>
</span>
</span>
<span
class="mx_TextualEvent mx_GenericEventListSummary_summary"
>
<span>
@user:id made no changes 100 times
</span>
</span>
</div>
</div>
</li>
</ol>
</div>
</div>
);
</DocumentFragment>
`;
exports[`MessagePanel should handle lots of room creation 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>
`;