Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix another freeze on room switch #7900

Merged
merged 2 commits into from
Feb 25, 2022
Merged

Fix another freeze on room switch #7900

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a3a4311
Fix another freeze on room switch
dbkr Feb 25, 2022
818de2b
Just bind to the currentstate state updates
dbkr Feb 25, 2022
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
80 changes: 25 additions & 55 deletions src/utils/permalinks/Permalinks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,8 @@ limitations under the License.
*/

import isIp from "is-ip";
import { throttle } from "lodash";
import * as utils from "matrix-js-sdk/src/utils";
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 { RoomStateEvent } from "matrix-js-sdk/src/models/room-state";

Expand Down Expand Up @@ -108,15 +104,9 @@ export class RoomPermalinkCreator {
if (!this.roomId) {
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) {
// 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.
Expand All @@ -125,38 +115,33 @@ export class RoomPermalinkCreator {
logger.warn("Tried to load a permalink creator with no room state");
return;
}
this.updateAllowedServers();
this.updateHighestPlUser();
this.updatePopulationMap();
this.updateServerCandidates();
this.fullUpdate();
}

start() {
public start() {
this.load();
this.room.client.on(RoomMemberEvent.Membership, this.onMembership);
this.room.currentState.on(RoomStateEvent.Events, this.onRoomState);
this.room.client.on(RoomStateEvent.Update, this.onRoomStateUpdate);
this.started = true;
}

stop() {
this.room.client.removeListener(RoomMemberEvent.Membership, this.onMembership);
this.room.currentState.removeListener(RoomStateEvent.Events, this.onRoomState);
public stop() {
this.room.client.removeListener(RoomStateEvent.Update, this.onRoomStateUpdate);
this.started = false;
}

get serverCandidates() {
public get serverCandidates() {
return this._serverCandidates;
}

isStarted() {
public isStarted() {
return this.started;
}

forEvent(eventId: string): string {
public forEvent(eventId: string): string {
return getPermalinkConstructor().forEvent(this.roomId, eventId, this._serverCandidates);
}

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

forRoom(): string {
public forRoom(): string {
return getPermalinkConstructor().forRoom(this.roomId, this._serverCandidates);
}

private onRoomState = (event: MatrixEvent) => {
switch (event.getType()) {
case EventType.RoomServerAcl:
this.updateAllowedServers();
this.updateHighestPlUser();
this.updatePopulationMap();
this.updateServerCandidates();
return;
case EventType.RoomPowerLevels:
this.updateHighestPlUser();
this.updateServerCandidates();
return;
}
private onRoomStateUpdate = () => {
this.fullUpdate();
t3chguy marked this conversation as resolved.
Show resolved Hide resolved
};

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]++;
}

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.updateHighestPlUser();
this.updatePopulationMap();
this.updateServerCandidates();
};
}

private updateHighestPlUser() {
const plEvent = this.room.currentState.getStateEvents("m.room.power_levels", "");
Expand Down
6 changes: 3 additions & 3 deletions test/utils/permalinks/Permalinks-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,14 @@ describe('Permalinks', function() {
},
member95,
]);
const creator = new RoomPermalinkCreator(room, null, false);
const creator = new RoomPermalinkCreator(room, null);
creator.load();
expect(creator._serverCandidates[0]).toBe("pl_95");
member95.membership = "left";
creator.onMembership({}, member95, "join");
creator.onRoomStateUpdate();
expect(creator._serverCandidates[0]).toBe("pl_75");
member95.membership = "join";
creator.onMembership({}, member95, "left");
creator.onRoomStateUpdate();
expect(creator._serverCandidates[0]).toBe("pl_95");
});

Expand Down