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

Commit 1a6134e

Browse files
authored
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 element-hq/element-web#21127 * Just bind to the currentstate state updates
1 parent 4ab5968 commit 1a6134e

File tree

2 files changed

+28
-58
lines changed

2 files changed

+28
-58
lines changed

src/utils/permalinks/Permalinks.ts

+25-55
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,8 @@ limitations under the License.
1515
*/
1616

1717
import isIp from "is-ip";
18-
import { throttle } from "lodash";
1918
import * as utils from "matrix-js-sdk/src/utils";
2019
import { Room } from "matrix-js-sdk/src/models/room";
21-
import { EventType } from "matrix-js-sdk/src/@types/event";
22-
import { MatrixEvent } from "matrix-js-sdk/src/models/event";
23-
import { RoomMember, RoomMemberEvent } from "matrix-js-sdk/src/models/room-member";
2420
import { logger } from "matrix-js-sdk/src/logger";
2521
import { RoomStateEvent } from "matrix-js-sdk/src/models/room-state";
2622

@@ -108,15 +104,9 @@ export class RoomPermalinkCreator {
108104
if (!this.roomId) {
109105
throw new Error("Failed to resolve a roomId for the permalink creator to use");
110106
}
111-
112-
if (shouldThrottle) {
113-
this.updateServerCandidates = throttle(
114-
this.updateServerCandidates, 200, { leading: true, trailing: true },
115-
);
116-
}
117107
}
118108

119-
load() {
109+
public load() {
120110
if (!this.room || !this.room.currentState) {
121111
// Under rare and unknown circumstances it is possible to have a room with no
122112
// currentState, at least potentially at the early stages of joining a room.
@@ -125,38 +115,33 @@ export class RoomPermalinkCreator {
125115
logger.warn("Tried to load a permalink creator with no room state");
126116
return;
127117
}
128-
this.updateAllowedServers();
129-
this.updateHighestPlUser();
130-
this.updatePopulationMap();
131-
this.updateServerCandidates();
118+
this.fullUpdate();
132119
}
133120

134-
start() {
121+
public start() {
135122
this.load();
136-
this.room.client.on(RoomMemberEvent.Membership, this.onMembership);
137-
this.room.currentState.on(RoomStateEvent.Events, this.onRoomState);
123+
this.room.currentState.on(RoomStateEvent.Update, this.onRoomStateUpdate);
138124
this.started = true;
139125
}
140126

141-
stop() {
142-
this.room.client.removeListener(RoomMemberEvent.Membership, this.onMembership);
143-
this.room.currentState.removeListener(RoomStateEvent.Events, this.onRoomState);
127+
public stop() {
128+
this.room.currentState.removeListener(RoomStateEvent.Update, this.onRoomStateUpdate);
144129
this.started = false;
145130
}
146131

147-
get serverCandidates() {
132+
public get serverCandidates() {
148133
return this._serverCandidates;
149134
}
150135

151-
isStarted() {
136+
public isStarted() {
152137
return this.started;
153138
}
154139

155-
forEvent(eventId: string): string {
140+
public forEvent(eventId: string): string {
156141
return getPermalinkConstructor().forEvent(this.roomId, eventId, this._serverCandidates);
157142
}
158143

159-
forShareableRoom(): string {
144+
public forShareableRoom(): string {
160145
if (this.room) {
161146
// Prefer to use canonical alias for permalink if possible
162147
const alias = this.room.getCanonicalAlias();
@@ -167,43 +152,28 @@ export class RoomPermalinkCreator {
167152
return getPermalinkConstructor().forRoom(this.roomId, this._serverCandidates);
168153
}
169154

170-
forRoom(): string {
155+
public forRoom(): string {
171156
return getPermalinkConstructor().forRoom(this.roomId, this._serverCandidates);
172157
}
173158

174-
private onRoomState = (event: MatrixEvent) => {
175-
switch (event.getType()) {
176-
case EventType.RoomServerAcl:
177-
this.updateAllowedServers();
178-
this.updateHighestPlUser();
179-
this.updatePopulationMap();
180-
this.updateServerCandidates();
181-
return;
182-
case EventType.RoomPowerLevels:
183-
this.updateHighestPlUser();
184-
this.updateServerCandidates();
185-
return;
186-
}
159+
private onRoomStateUpdate = () => {
160+
this.fullUpdate();
187161
};
188162

189-
private onMembership = (evt: MatrixEvent, member: RoomMember, oldMembership: string) => {
190-
if (member.roomId !== this.room.roomId) return;
191-
192-
const userId = member.userId;
193-
const membership = member.membership;
194-
const serverName = getServerName(userId);
195-
const hasJoined = oldMembership !== "join" && membership === "join";
196-
const hasLeft = oldMembership === "join" && membership !== "join";
197-
198-
if (hasLeft) {
199-
this.populationMap[serverName]--;
200-
} else if (hasJoined) {
201-
this.populationMap[serverName]++;
202-
}
203-
163+
private fullUpdate() {
164+
// This updates the internal state of this object from the room state. It's broken
165+
// down into separate functions, previously because we did some of these as incremental
166+
// updates, but they were on member events which can be very numerous, so the incremental
167+
// updates ended up being much slower than a full update. We now have the batch state update
168+
// event, so we just update in full, but on each batch of updates.
169+
// A full update takes about 120ms for me on Matrix HQ, which still feels like way too long
170+
// to be spending worrying about how we might generate a permalink, but it's better than
171+
// multiple seconds.
172+
this.updateAllowedServers();
204173
this.updateHighestPlUser();
174+
this.updatePopulationMap();
205175
this.updateServerCandidates();
206-
};
176+
}
207177

208178
private updateHighestPlUser() {
209179
const plEvent = this.room.currentState.getStateEvents("m.room.power_levels", "");

test/utils/permalinks/Permalinks-test.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -122,14 +122,14 @@ describe('Permalinks', function() {
122122
},
123123
member95,
124124
]);
125-
const creator = new RoomPermalinkCreator(room, null, false);
125+
const creator = new RoomPermalinkCreator(room, null);
126126
creator.load();
127127
expect(creator._serverCandidates[0]).toBe("pl_95");
128128
member95.membership = "left";
129-
creator.onMembership({}, member95, "join");
129+
creator.onRoomStateUpdate();
130130
expect(creator._serverCandidates[0]).toBe("pl_75");
131131
member95.membership = "join";
132-
creator.onMembership({}, member95, "left");
132+
creator.onRoomStateUpdate();
133133
expect(creator._serverCandidates[0]).toBe("pl_95");
134134
});
135135

0 commit comments

Comments
 (0)