Fix another freeze on room switch (#7900)

* Fix another freeze on room switch

This switches permalinks to use the batch state update event and
removes the incremental updates, as commented. We now spend, on my
profiling, about 450ms in setOutOfBandMembers itself and another
120ms in permalinks.

Fixes https://github.com/vector-im/element-web/issues/21127

* Just bind to the currentstate state updates
This commit is contained in:
David Baker 2022-02-25 19:48:35 +00:00 committed by GitHub
parent 4ab59684c1
commit 1a6134e441
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 28 additions and 58 deletions

View file

@ -15,12 +15,8 @@ limitations under the License.
*/ */
import isIp from "is-ip"; import isIp from "is-ip";
import { throttle } from "lodash";
import * as utils from "matrix-js-sdk/src/utils"; import * as utils from "matrix-js-sdk/src/utils";
import { Room } from "matrix-js-sdk/src/models/room"; import { Room } from "matrix-js-sdk/src/models/room";
import { EventType } from "matrix-js-sdk/src/@types/event";
import { MatrixEvent } from "matrix-js-sdk/src/models/event";
import { RoomMember, RoomMemberEvent } from "matrix-js-sdk/src/models/room-member";
import { logger } from "matrix-js-sdk/src/logger"; import { logger } from "matrix-js-sdk/src/logger";
import { RoomStateEvent } from "matrix-js-sdk/src/models/room-state"; import { RoomStateEvent } from "matrix-js-sdk/src/models/room-state";
@ -108,15 +104,9 @@ export class RoomPermalinkCreator {
if (!this.roomId) { if (!this.roomId) {
throw new Error("Failed to resolve a roomId for the permalink creator to use"); throw new Error("Failed to resolve a roomId for the permalink creator to use");
} }
if (shouldThrottle) {
this.updateServerCandidates = throttle(
this.updateServerCandidates, 200, { leading: true, trailing: true },
);
}
} }
load() { public load() {
if (!this.room || !this.room.currentState) { if (!this.room || !this.room.currentState) {
// Under rare and unknown circumstances it is possible to have a room with no // Under rare and unknown circumstances it is possible to have a room with no
// currentState, at least potentially at the early stages of joining a room. // currentState, at least potentially at the early stages of joining a room.
@ -125,38 +115,33 @@ export class RoomPermalinkCreator {
logger.warn("Tried to load a permalink creator with no room state"); logger.warn("Tried to load a permalink creator with no room state");
return; return;
} }
this.updateAllowedServers(); this.fullUpdate();
this.updateHighestPlUser();
this.updatePopulationMap();
this.updateServerCandidates();
} }
start() { public start() {
this.load(); this.load();
this.room.client.on(RoomMemberEvent.Membership, this.onMembership); this.room.currentState.on(RoomStateEvent.Update, this.onRoomStateUpdate);
this.room.currentState.on(RoomStateEvent.Events, this.onRoomState);
this.started = true; this.started = true;
} }
stop() { public stop() {
this.room.client.removeListener(RoomMemberEvent.Membership, this.onMembership); this.room.currentState.removeListener(RoomStateEvent.Update, this.onRoomStateUpdate);
this.room.currentState.removeListener(RoomStateEvent.Events, this.onRoomState);
this.started = false; this.started = false;
} }
get serverCandidates() { public get serverCandidates() {
return this._serverCandidates; return this._serverCandidates;
} }
isStarted() { public isStarted() {
return this.started; return this.started;
} }
forEvent(eventId: string): string { public forEvent(eventId: string): string {
return getPermalinkConstructor().forEvent(this.roomId, eventId, this._serverCandidates); return getPermalinkConstructor().forEvent(this.roomId, eventId, this._serverCandidates);
} }
forShareableRoom(): string { public forShareableRoom(): string {
if (this.room) { if (this.room) {
// Prefer to use canonical alias for permalink if possible // Prefer to use canonical alias for permalink if possible
const alias = this.room.getCanonicalAlias(); const alias = this.room.getCanonicalAlias();
@ -167,43 +152,28 @@ export class RoomPermalinkCreator {
return getPermalinkConstructor().forRoom(this.roomId, this._serverCandidates); return getPermalinkConstructor().forRoom(this.roomId, this._serverCandidates);
} }
forRoom(): string { public forRoom(): string {
return getPermalinkConstructor().forRoom(this.roomId, this._serverCandidates); return getPermalinkConstructor().forRoom(this.roomId, this._serverCandidates);
} }
private onRoomState = (event: MatrixEvent) => { private onRoomStateUpdate = () => {
switch (event.getType()) { this.fullUpdate();
case EventType.RoomServerAcl: };
private fullUpdate() {
// This updates the internal state of this object from the room state. It's broken
// down into separate functions, previously because we did some of these as incremental
// updates, but they were on member events which can be very numerous, so the incremental
// updates ended up being much slower than a full update. We now have the batch state update
// event, so we just update in full, but on each batch of updates.
// A full update takes about 120ms for me on Matrix HQ, which still feels like way too long
// to be spending worrying about how we might generate a permalink, but it's better than
// multiple seconds.
this.updateAllowedServers(); this.updateAllowedServers();
this.updateHighestPlUser(); this.updateHighestPlUser();
this.updatePopulationMap(); this.updatePopulationMap();
this.updateServerCandidates(); this.updateServerCandidates();
return;
case EventType.RoomPowerLevels:
this.updateHighestPlUser();
this.updateServerCandidates();
return;
} }
};
private onMembership = (evt: MatrixEvent, member: RoomMember, oldMembership: string) => {
if (member.roomId !== this.room.roomId) return;
const userId = member.userId;
const membership = member.membership;
const serverName = getServerName(userId);
const hasJoined = oldMembership !== "join" && membership === "join";
const hasLeft = oldMembership === "join" && membership !== "join";
if (hasLeft) {
this.populationMap[serverName]--;
} else if (hasJoined) {
this.populationMap[serverName]++;
}
this.updateHighestPlUser();
this.updateServerCandidates();
};
private updateHighestPlUser() { private updateHighestPlUser() {
const plEvent = this.room.currentState.getStateEvents("m.room.power_levels", ""); const plEvent = this.room.currentState.getStateEvents("m.room.power_levels", "");

View file

@ -122,14 +122,14 @@ describe('Permalinks', function() {
}, },
member95, member95,
]); ]);
const creator = new RoomPermalinkCreator(room, null, false); const creator = new RoomPermalinkCreator(room, null);
creator.load(); creator.load();
expect(creator._serverCandidates[0]).toBe("pl_95"); expect(creator._serverCandidates[0]).toBe("pl_95");
member95.membership = "left"; member95.membership = "left";
creator.onMembership({}, member95, "join"); creator.onRoomStateUpdate();
expect(creator._serverCandidates[0]).toBe("pl_75"); expect(creator._serverCandidates[0]).toBe("pl_75");
member95.membership = "join"; member95.membership = "join";
creator.onMembership({}, member95, "left"); creator.onRoomStateUpdate();
expect(creator._serverCandidates[0]).toBe("pl_95"); expect(creator._serverCandidates[0]).toBe("pl_95");
}); });