PrivateCoffee/element-web
4
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 3

Improve room switching in the new room list

Browse source 
For https://github.com/vector-im/riot-web/issues/14034

One of the largest issues with room switching was that we'd regenerate the entire list when the sticky room changes, which is obviously detrimental on larger accounts (and even some medium accounts). To work through this, we simply handle the NewRoom and RoomRemoved causes (used by the sticky room handling) as splices rather than in-place updates.

Overall this leads to a smoother experience as it means we're doing far less calculations and can even opt out of an update if it isn't required, such as a RoomRemoved cause being fired twice - the second one can result in an update not being required, saving render time.

This commit also includes a fix for handling update causes on the sticky room, as the room list loves to print errors when this happens. We don't need to handle any updates because once the sticky room changes it'll get re-added through NewRoom, causing the underlying algorithm to slot it in where needed, effectively handling all the missed updates.
This commit is contained in:
Travis Ralston 2020-06-17 22:42:01 -06:00
parent 803b7bb30f
commit 8db67743f7
4 changed files with 84 additions and 36 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
src/stores/room-list/algorithms/Algorithm.ts
View file

@ -508,16 +508,14 @@ export class Algorithm extends EventEmitter {
return true;
}
if (cause === RoomUpdateCause.NewRoom) {
// TODO: Be smarter and insert rather than regen the planet.
await this.setKnownRooms([room, ...this.rooms]);
return true;
// If the update is for a room change which might be the sticky room, prevent it. We
// need to make sure that the causes (NewRoom and RoomRemoved) are still triggered though
// as the sticky room relies on this.
if (cause !== RoomUpdateCause.NewRoom && cause !== RoomUpdateCause.RoomRemoved) {
if (this.stickyRoom === room) {
console.warn(`[RoomListDebug] Received ${cause} update for sticky room ${room.roomId} - ignoring`);
return false;
}
if (cause === RoomUpdateCause.RoomRemoved) {
// TODO: Be smarter and splice rather than regen the planet.
await this.setKnownRooms(this.rooms.filter(r => r !== room));
return true;
}
let tags = this.roomIdsToTags[room.roomId];

83
src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts
View file

@ -87,7 +87,7 @@ export class ImportanceAlgorithm extends OrderingAlgorithm {
public constructor(tagId: TagID, initialSortingAlgorithm: SortAlgorithm) {
super(tagId, initialSortingAlgorithm);
console.log("Constructed an ImportanceAlgorithm");
console.log(`[RoomListDebug] Constructed an ImportanceAlgorithm for ${tagId}`);
}
// noinspection JSMethodCanBeStatic
@ -151,8 +151,36 @@ export class ImportanceAlgorithm extends OrderingAlgorithm {
}
}
private async handleSplice(room: Room, cause: RoomUpdateCause): Promise<boolean> {
if (cause === RoomUpdateCause.NewRoom) {
const category = this.getRoomCategory(room);
this.alterCategoryPositionBy(category, 1, this.indices);
this.cachedOrderedRooms.splice(this.indices[category], 0, room); // splice in the new room (pre-adjusted)
} else if (cause === RoomUpdateCause.RoomRemoved) {
const roomIdx = this.getRoomIndex(room);
if (roomIdx === -1) return false; // no change
const oldCategory = this.getCategoryFromIndices(roomIdx, this.indices);
this.alterCategoryPositionBy(oldCategory, -1, this.indices);
this.cachedOrderedRooms.splice(roomIdx, 1); // remove the room
} else {
throw new Error(`Unhandled splice: ${cause}`);
}
}
private getRoomIndex(room: Room): number {
let roomIdx = this.cachedOrderedRooms.indexOf(room);
if (roomIdx === -1) { // can only happen if the js-sdk's store goes sideways.
console.warn(`Degrading performance to find missing room in "${this.tagId}": ${room.roomId}`);
roomIdx = this.cachedOrderedRooms.findIndex(r => r.roomId === room.roomId);
}
return roomIdx;
}
public async handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise<boolean> {
// TODO: Handle NewRoom and RoomRemoved
if (cause === RoomUpdateCause.NewRoom || cause === RoomUpdateCause.RoomRemoved) {
return this.handleSplice(room, cause);
}
if (cause !== RoomUpdateCause.Timeline && cause !== RoomUpdateCause.ReadReceipt) {
throw new Error(`Unsupported update cause: ${cause}`);
}
@ -162,11 +190,7 @@ export class ImportanceAlgorithm extends OrderingAlgorithm {
return; // Nothing to do here.
}
let roomIdx = this.cachedOrderedRooms.indexOf(room);
if (roomIdx === -1) { // can only happen if the js-sdk's store goes sideways.
console.warn(`Degrading performance to find missing room in "${this.tagId}": ${room.roomId}`);
roomIdx = this.cachedOrderedRooms.findIndex(r => r.roomId === room.roomId);
}
const roomIdx = this.getRoomIndex(room);
if (roomIdx === -1) {
throw new Error(`Room ${room.roomId} has no index in ${this.tagId}`);
}
@ -188,12 +212,18 @@ export class ImportanceAlgorithm extends OrderingAlgorithm {
// room from the array.
}
// The room received an update, so take out the slice and sort it. This should be relatively
// quick because the room is inserted at the top of the category, and most popular sorting
// algorithms will deal with trying to keep the active room at the top/start of the category.
// For the few algorithms that will have to move the thing quite far (alphabetic with a Z room
// for example), the list should already be sorted well enough that it can rip through the
// array and slot the changed room in quickly.
// Sort the category now that we've dumped the room in
await this.sortCategory(category);
return true; // change made
}
private async sortCategory(category: Category) {
// This should be relatively quick because the room is usually inserted at the top of the
// category, and most popular sorting algorithms will deal with trying to keep the active
// room at the top/start of the category. For the few algorithms that will have to move the
// thing quite far (alphabetic with a Z room for example), the list should already be sorted
// well enough that it can rip through the array and slot the changed room in quickly.
const nextCategoryStartIdx = category === CATEGORY_ORDER[CATEGORY_ORDER.length - 1]
? Number.MAX_SAFE_INTEGER
: this.indices[CATEGORY_ORDER[CATEGORY_ORDER.indexOf(category) + 1]];
@ -202,8 +232,6 @@ export class ImportanceAlgorithm extends OrderingAlgorithm {
const unsortedSlice = this.cachedOrderedRooms.splice(startIdx, numSort);
const sorted = await sortRoomsWithAlgorithm(unsortedSlice, this.tagId, this.sortingAlgorithm);
this.cachedOrderedRooms.splice(startIdx, 0, ...sorted);
return true; // change made
}
// noinspection JSMethodCanBeStatic
@ -230,14 +258,29 @@ export class ImportanceAlgorithm extends OrderingAlgorithm {
// We also need to update subsequent categories as they'll all shift by nRooms, so we
// loop over the order to achieve that.
for (let i = CATEGORY_ORDER.indexOf(fromCategory) + 1; i < CATEGORY_ORDER.length; i++) {
const nextCategory = CATEGORY_ORDER[i];
indices[nextCategory] -= nRooms;
this.alterCategoryPositionBy(fromCategory, -nRooms, indices);
this.alterCategoryPositionBy(toCategory, +nRooms, indices);
}
for (let i = CATEGORY_ORDER.indexOf(toCategory) + 1; i < CATEGORY_ORDER.length; i++) {
private alterCategoryPositionBy(category: Category, n: number, indices: ICategoryIndex) {
// Note: when we alter a category's index, we actually have to modify the ones following
// the target and not the target itself.
// XXX: If this ever actually gets more than one room passed to it, it'll need more index
// handling. For instance, if 45 rooms are removed from the middle of a 50 room list, the
// index for the categories will be way off.
const nextOrderIndex = CATEGORY_ORDER.indexOf(category) + 1
if (n > 0) {
for (let i = nextOrderIndex; i < CATEGORY_ORDER.length; i++) {
const nextCategory = CATEGORY_ORDER[i];
indices[nextCategory] += nRooms;
indices[nextCategory] += Math.abs(n);
}
} else if (n < 0) {
for (let i = nextOrderIndex; i < CATEGORY_ORDER.length; i++) {
const nextCategory = CATEGORY_ORDER[i];
indices[nextCategory] -= Math.abs(n);
}
}
// Do a quick check to see if we've completely broken the index

14
src/stores/room-list/algorithms/list-ordering/NaturalAlgorithm.ts
View file

@ -28,7 +28,7 @@ export class NaturalAlgorithm extends OrderingAlgorithm {
public constructor(tagId: TagID, initialSortingAlgorithm: SortAlgorithm) {
super(tagId, initialSortingAlgorithm);
console.log("Constructed a NaturalAlgorithm");
console.log(`[RoomListDebug] Constructed a NaturalAlgorithm for ${tagId}`);
}
public async setRooms(rooms: Room[]): Promise<any> {
@ -36,11 +36,19 @@ export class NaturalAlgorithm extends OrderingAlgorithm {
}
public async handleRoomUpdate(room, cause): Promise<boolean> {
// TODO: Handle NewRoom and RoomRemoved
if (cause !== RoomUpdateCause.Timeline && cause !== RoomUpdateCause.ReadReceipt) {
const isSplice = cause === RoomUpdateCause.NewRoom || cause === RoomUpdateCause.RoomRemoved;
const isInPlace = cause === RoomUpdateCause.Timeline || cause === RoomUpdateCause.ReadReceipt;
if (!isSplice && !isInPlace) {
throw new Error(`Unsupported update cause: ${cause}`);
}
if (cause === RoomUpdateCause.NewRoom) {
this.cachedOrderedRooms.push(room);
} else if (cause === RoomUpdateCause.RoomRemoved) {
const idx = this.cachedOrderedRooms.indexOf(room);
if (idx >= 0) this.cachedOrderedRooms.splice(idx, 1);
}
// TODO: Optimize this to avoid useless operations
// For example, we can skip updates to alphabetic (sometimes) and manually ordered tags
this.cachedOrderedRooms = await sortRoomsWithAlgorithm(this.cachedOrderedRooms, this.tagId, this.sortingAlgorithm);

1
src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts
View file

@ -67,6 +67,5 @@ export abstract class OrderingAlgorithm {
* @param cause The cause of the update.
* @returns True if the update requires the Algorithm to update the presentation layers.
*/
// XXX: TODO: We assume this will only ever be a position update and NOT a NewRoom or RemoveRoom change!!
public abstract handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise<boolean>;
}