Merge pull request #5038 from matrix-org/travis/perf2

Performance improvements round 2: Maps, freezing, dispatching, and flexbox obliteration

res/css/structures/_LeftPanel.scss

@@ -135,12 +135,7 @@ $tagPanelWidth: 56px; // only applies in this file, used for calculations
         }
 
         .mx_LeftPanel_roomListWrapper {
-            // Create a flexbox to ensure the containing items cause appropriate overflow.
-            display: flex;
-
-            flex-grow: 1;
             overflow: hidden;
-            min-height: 0;
             margin-top: 10px; // so we're not up against the search/filter
 
             &.mx_LeftPanel_roomListWrapper_stickyBottom {
@@ -153,14 +148,8 @@ $tagPanelWidth: 56px; // only applies in this file, used for calculations
         }
 
         .mx_LeftPanel_actualRoomListContainer {
-            flex-grow: 1; // fill the available space
-            overflow-y: auto;
-            width: 100%;
-            max-width: 100%;
             position: relative; // for sticky headers
-
+            height: 100%; // ensure scrolling still works
-            // Create a flexbox to trick the layout engine
-            display: flex;
         }
     }
 

res/css/views/rooms/_RoomList.scss

@@ -15,11 +15,5 @@ limitations under the License.
 */
 
 .mx_RoomList {
-    width: calc(100% - 16px); // 16px of artificial right-side margin (8px is overflowed from the sublists)
+    padding-right: 7px; // width of the scrollbar, to line things up
-
-    // Create a column-based flexbox for the sublists. That's pretty much all we have to
-    // worry about in this stylesheet.
-    display: flex;
-    flex-direction: column;
-    flex-wrap: nowrap; // let the column overflow
 }

res/css/views/rooms/_RoomSublist.scss

@@ -15,15 +15,8 @@ limitations under the License.
 */
 
 .mx_RoomSublist {
-    // The sublist is a column of rows, essentially
-    display: flex;
-    flex-direction: column;
-
     margin-left: 8px;
     margin-bottom: 4px;
-    width: 100%;
-
-    flex-shrink: 0; // to convince safari's layout engine the flexbox is fine
 
     .mx_RoomSublist_headerContainer {
         // Create a flexbox to make alignment easy

src/stores/AsyncStore.ts

@@ -42,7 +42,7 @@ export const UPDATE_EVENT = "update";
  * help prevent lock conflicts.
  */
 export abstract class AsyncStore<T extends Object> extends EventEmitter {
-    private storeState: T;
+    private storeState: Readonly<T>;
     private lock = new AwaitLock();
     private readonly dispatcherRef: string;
 
@@ -62,7 +62,7 @@ export abstract class AsyncStore<T extends Object> extends EventEmitter {
      * The current state of the store. Cannot be mutated.
      */
     protected get state(): T {
-        return Object.freeze(this.storeState);
+        return this.storeState;
     }
 
     /**
@@ -79,7 +79,7 @@ export abstract class AsyncStore<T extends Object> extends EventEmitter {
     protected async updateState(newState: T | Object) {
         await this.lock.acquireAsync();
         try {
-            this.storeState = Object.assign(<T>{}, this.storeState, newState);
+            this.storeState = Object.freeze(Object.assign(<T>{}, this.storeState, newState));
             this.emit(UPDATE_EVENT, this);
         } finally {
             await this.lock.release();
@@ -94,7 +94,7 @@ export abstract class AsyncStore<T extends Object> extends EventEmitter {
     protected async reset(newState: T | Object = null, quiet = false) {
         await this.lock.acquireAsync();
         try {
-            this.storeState = <T>(newState || {});
+            this.storeState = Object.freeze(<T>(newState || {}));
             if (!quiet) this.emit(UPDATE_EVENT, this);
         } finally {
             await this.lock.release();

src/stores/room-list/MessagePreviewStore.ts

@@ -26,6 +26,7 @@ import { CallAnswerEventPreview } from "./previews/CallAnswerEventPreview";
 import { CallHangupEvent } from "./previews/CallHangupEvent";
 import { StickerEventPreview } from "./previews/StickerEventPreview";
 import { ReactionEventPreview } from "./previews/ReactionEventPreview";
+import { UPDATE_EVENT } from "../AsyncStore";
 
 const PREVIEWS = {
     'm.room.message': {
@@ -62,12 +63,15 @@ type TAG_ANY = "im.vector.any";
 const TAG_ANY: TAG_ANY = "im.vector.any";
 
 interface IState {
-    [roomId: string]: Map<TagID | TAG_ANY, string | null>; // null indicates the preview is empty / irrelevant
+    // Empty because we don't actually use the state
 }
 
 export class MessagePreviewStore extends AsyncStoreWithClient<IState> {
     private static internalInstance = new MessagePreviewStore();
 
+    // null indicates the preview is empty / irrelevant
+    private previews = new Map<string, Map<TagID|TAG_ANY, string|null>>();
+
     private constructor() {
         super(defaultDispatcher, {});
     }
@@ -85,10 +89,9 @@ export class MessagePreviewStore extends AsyncStoreWithClient<IState> {
     public getPreviewForRoom(room: Room, inTagId: TagID): string {
         if (!room) return null; // invalid room, just return nothing
 
-        const val = this.state[room.roomId];
+        if (!this.previews.has(room.roomId)) this.generatePreview(room, inTagId);
-        if (!val) this.generatePreview(room, inTagId);
 
-        const previews = this.state[room.roomId];
+        const previews = this.previews.get(room.roomId);
         if (!previews) return null;
 
         if (!previews.has(inTagId)) {
@@ -101,11 +104,10 @@ export class MessagePreviewStore extends AsyncStoreWithClient<IState> {
         const events = room.timeline;
         if (!events) return; // should only happen in tests
 
-        let map = this.state[room.roomId];
+        let map = this.previews.get(room.roomId);
         if (!map) {
             map = new Map<TagID | TAG_ANY, string | null>();
-
+            this.previews.set(room.roomId, map);
-            // We set the state later with the map, so no need to send an update now
         }
 
         // Set the tags so we know what to generate
@@ -141,16 +143,16 @@ export class MessagePreviewStore extends AsyncStoreWithClient<IState> {
             }
 
             if (changed) {
-                // Update state for good measure - causes emit for update
+                // We've muted the underlying Map, so just emit that we've changed.
-                // noinspection JSIgnoredPromiseFromCall - the AsyncStore handles concurrent calls
+                this.previews.set(room.roomId, map);
-                this.updateState({[room.roomId]: map});
+                this.emit(UPDATE_EVENT, this);
             }
             return; // we're done
         }
 
         // At this point, we didn't generate a preview so clear it
-        // noinspection JSIgnoredPromiseFromCall - the AsyncStore handles concurrent calls
+        this.previews.set(room.roomId, new Map<TagID|TAG_ANY, string|null>());
-        this.updateState({[room.roomId]: null});
+        this.emit(UPDATE_EVENT, this);
     }
 
     protected async onAction(payload: ActionPayload) {
@@ -158,7 +160,7 @@ export class MessagePreviewStore extends AsyncStoreWithClient<IState> {
 
         if (payload.action === 'MatrixActions.Room.timeline' || payload.action === 'MatrixActions.Event.decrypted') {
             const event = payload.event; // TODO: Type out the dispatcher
-            if (!Object.keys(this.state).includes(event.getRoomId())) return; // not important
+            if (!this.previews.has(event.getRoomId())) return; // not important
             this.generatePreview(this.matrixClient.getRoom(event.getRoomId()), TAG_ANY);
         }
     }

src/stores/room-list/RoomListStore.ts

@@ -168,6 +168,12 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> {
     }
 
     protected async onAction(payload: ActionPayload) {
+        // If we're not remotely ready, don't even bother scheduling the dispatch handling.
+        // This is repeated in the handler just in case things change between a decision here and
+        // when the timer fires.
+        const logicallyReady = this.matrixClient && this.initialListsGenerated;
+        if (!logicallyReady) return;
+
         // When we're running tests we can't reliably use setImmediate out of timing concerns.
         // As such, we use a more synchronous model.
         if (RoomListStoreClass.TEST_MODE) {