Merge pull request #6063 from matrix-org/t3chguy/fix/17369

Move SettingsStore watchers/monitors over to ES6 maps for performance
This commit is contained in:
Michael Telatynski 2021-05-25 10:58:09 +01:00 committed by GitHub
commit 97c6ee39d0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 36 additions and 39 deletions

View file

@ -26,7 +26,7 @@ import { _t } from '../languageHandler';
import dis from '../dispatcher/dispatcher'; import dis from '../dispatcher/dispatcher';
import { ISetting, SETTINGS } from "./Settings"; import { ISetting, SETTINGS } from "./Settings";
import LocalEchoWrapper from "./handlers/LocalEchoWrapper"; import LocalEchoWrapper from "./handlers/LocalEchoWrapper";
import { WatchManager } from "./WatchManager"; import { WatchManager, CallbackFn as WatchCallbackFn } from "./WatchManager";
import { SettingLevel } from "./SettingLevel"; import { SettingLevel } from "./SettingLevel";
import SettingsHandler from "./handlers/SettingsHandler"; import SettingsHandler from "./handlers/SettingsHandler";
@ -117,8 +117,8 @@ export default class SettingsStore {
// We also maintain a list of monitors which are special watchers: they cause dispatches // We also maintain a list of monitors which are special watchers: they cause dispatches
// when the setting changes. We track which rooms we're monitoring though to ensure we // when the setting changes. We track which rooms we're monitoring though to ensure we
// don't duplicate updates on the bus. // don't duplicate updates on the bus.
private static watchers = {}; // { callbackRef => { callbackFn } } private static watchers = new Map<string, WatchCallbackFn>();
private static monitors = {}; // { settingName => { roomId => callbackRef } } private static monitors = new Map<string, Map<string, string>>(); // { settingName => { roomId => callbackRef } }
// Counter used for generation of watcher IDs // Counter used for generation of watcher IDs
private static watcherCount = 1; private static watcherCount = 1;
@ -163,7 +163,7 @@ export default class SettingsStore {
callbackFn(originalSettingName, changedInRoomId, atLevel, newValAtLevel, newValue); callbackFn(originalSettingName, changedInRoomId, atLevel, newValAtLevel, newValue);
}; };
SettingsStore.watchers[watcherId] = localizedCallback; SettingsStore.watchers.set(watcherId, localizedCallback);
defaultWatchManager.watchSetting(settingName, roomId, localizedCallback); defaultWatchManager.watchSetting(settingName, roomId, localizedCallback);
return watcherId; return watcherId;
@ -176,13 +176,13 @@ export default class SettingsStore {
* to cancel. * to cancel.
*/ */
public static unwatchSetting(watcherReference: string) { public static unwatchSetting(watcherReference: string) {
if (!SettingsStore.watchers[watcherReference]) { if (!SettingsStore.watchers.has(watcherReference)) {
console.warn(`Ending non-existent watcher ID ${watcherReference}`); console.warn(`Ending non-existent watcher ID ${watcherReference}`);
return; return;
} }
defaultWatchManager.unwatchSetting(SettingsStore.watchers[watcherReference]); defaultWatchManager.unwatchSetting(SettingsStore.watchers.get(watcherReference));
delete SettingsStore.watchers[watcherReference]; SettingsStore.watchers.delete(watcherReference);
} }
/** /**
@ -196,10 +196,10 @@ export default class SettingsStore {
public static monitorSetting(settingName: string, roomId: string) { public static monitorSetting(settingName: string, roomId: string) {
roomId = roomId || null; // the thing wants null specifically to work, so appease it. roomId = roomId || null; // the thing wants null specifically to work, so appease it.
if (!this.monitors[settingName]) this.monitors[settingName] = {}; if (!this.monitors.has(settingName)) this.monitors.set(settingName, new Map());
const registerWatcher = () => { const registerWatcher = () => {
this.monitors[settingName][roomId] = SettingsStore.watchSetting( this.monitors.get(settingName).set(roomId, SettingsStore.watchSetting(
settingName, roomId, (settingName, inRoomId, level, newValueAtLevel, newValue) => { settingName, roomId, (settingName, inRoomId, level, newValueAtLevel, newValue) => {
dis.dispatch({ dis.dispatch({
action: 'setting_updated', action: 'setting_updated',
@ -210,19 +210,20 @@ export default class SettingsStore {
newValue, newValue,
}); });
}, },
); ));
}; };
const hasRoom = Object.keys(this.monitors[settingName]).find((r) => r === roomId || r === null); const rooms = Array.from(this.monitors.get(settingName).keys());
const hasRoom = rooms.find((r) => r === roomId || r === null);
if (!hasRoom) { if (!hasRoom) {
registerWatcher(); registerWatcher();
} else { } else {
if (roomId === null) { if (roomId === null) {
// Unregister all existing watchers and register the new one // Unregister all existing watchers and register the new one
for (const roomId of Object.keys(this.monitors[settingName])) { rooms.forEach(roomId => {
SettingsStore.unwatchSetting(this.monitors[settingName][roomId]); SettingsStore.unwatchSetting(this.monitors.get(settingName).get(roomId));
} });
this.monitors[settingName] = {}; this.monitors.get(settingName).clear();
registerWatcher(); registerWatcher();
} // else a watcher is already registered for the room, so don't bother registering it again } // else a watcher is already registered for the room, so don't bother registering it again
} }

View file

@ -18,11 +18,7 @@ import { SettingLevel } from "./SettingLevel";
export type CallbackFn = (changedInRoomId: string, atLevel: SettingLevel, newValAtLevel: any) => void; export type CallbackFn = (changedInRoomId: string, atLevel: SettingLevel, newValAtLevel: any) => void;
const IRRELEVANT_ROOM: string = null; const IRRELEVANT_ROOM = Symbol("irrelevant-room");
interface RoomWatcherMap {
[roomId: string]: CallbackFn[];
}
/** /**
* Generalized management class for dealing with watchers on a per-handler (per-level) * Generalized management class for dealing with watchers on a per-handler (per-level)
@ -30,25 +26,25 @@ interface RoomWatcherMap {
* class, which are then proxied outwards to any applicable watchers. * class, which are then proxied outwards to any applicable watchers.
*/ */
export class WatchManager { export class WatchManager {
private watchers: {[settingName: string]: RoomWatcherMap} = {}; private watchers = new Map<string, Map<string | symbol, CallbackFn[]>>(); // settingName -> roomId -> CallbackFn[]
// Proxy for handlers to delegate changes to this manager // Proxy for handlers to delegate changes to this manager
public watchSetting(settingName: string, roomId: string | null, cb: CallbackFn) { public watchSetting(settingName: string, roomId: string | null, cb: CallbackFn) {
if (!this.watchers[settingName]) this.watchers[settingName] = {}; if (!this.watchers.has(settingName)) this.watchers.set(settingName, new Map());
if (!this.watchers[settingName][roomId]) this.watchers[settingName][roomId] = []; if (!this.watchers.get(settingName).has(roomId)) this.watchers.get(settingName).set(roomId, []);
this.watchers[settingName][roomId].push(cb); this.watchers.get(settingName).get(roomId).push(cb);
} }
// Proxy for handlers to delegate changes to this manager // Proxy for handlers to delegate changes to this manager
public unwatchSetting(cb: CallbackFn) { public unwatchSetting(cb: CallbackFn) {
for (const settingName of Object.keys(this.watchers)) { this.watchers.forEach((map) => {
for (const roomId of Object.keys(this.watchers[settingName])) { map.forEach((callbacks) => {
let idx; let idx;
while ((idx = this.watchers[settingName][roomId].indexOf(cb)) !== -1) { while ((idx = callbacks.indexOf(cb)) !== -1) {
this.watchers[settingName][roomId].splice(idx, 1); callbacks.splice(idx, 1);
} }
} });
} });
} }
public notifyUpdate(settingName: string, inRoomId: string | null, atLevel: SettingLevel, newValueAtLevel: any) { public notifyUpdate(settingName: string, inRoomId: string | null, atLevel: SettingLevel, newValueAtLevel: any) {
@ -56,21 +52,21 @@ export class WatchManager {
// we also don't have a reliable way to get the old value of a setting. Instead, we'll just // we also don't have a reliable way to get the old value of a setting. Instead, we'll just
// let it fall through regardless and let the receiver dedupe if they want to. // let it fall through regardless and let the receiver dedupe if they want to.
if (!this.watchers[settingName]) return; if (!this.watchers.has(settingName)) return;
const roomWatchers = this.watchers[settingName]; const roomWatchers = this.watchers.get(settingName);
const callbacks = []; const callbacks = [];
if (inRoomId !== null && roomWatchers[inRoomId]) { if (inRoomId !== null && roomWatchers.has(inRoomId)) {
callbacks.push(...roomWatchers[inRoomId]); callbacks.push(...roomWatchers.get(inRoomId));
} }
if (!inRoomId) { if (!inRoomId) {
// Fire updates to all the individual room watchers too, as they probably // Fire updates to all the individual room watchers too, as they probably care about the change higher up.
// care about the change higher up. const callbacks = Array.from(roomWatchers.values()).flat(1);
callbacks.push(...Object.values(roomWatchers).flat(1)); callbacks.push(...callbacks);
} else if (roomWatchers[IRRELEVANT_ROOM]) { } else if (roomWatchers.has(IRRELEVANT_ROOM)) {
callbacks.push(...roomWatchers[IRRELEVANT_ROOM]); callbacks.push(...roomWatchers.get(IRRELEVANT_ROOM));
} }
for (const callback of callbacks) { for (const callback of callbacks) {