ux: bug fix: memory leak in the connections tab

staltz · Oct 26, 2022 · d70b6cd · d70b6cd
1 parent d5095ba
commit d70b6cd
Show file tree

Hide file tree

Showing 8 changed files with 242 additions and 126 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -96,6 +96,7 @@
     "cycle-native-toastandroid": "1.1.0",
     "electron-context-menu": "3.1.2",
     "electron-ssb-client": "2.0.0",
+    "fast-deep-equal": "3.1.3",
     "fix-webm-duration": "1.0.4",
     "hermes-engine": "0.9.0",
     "i18n-js": "3.8.0",

diff --git a/src/frontend/screens/central/connections-tab/model.ts b/src/frontend/screens/central/connections-tab/model.ts
@@ -6,6 +6,7 @@ import xs, {Stream} from 'xstream';
 import sample from 'xstream-sample';
 import concat from 'xstream/extra/concat';
 import {FeedId} from 'ssb-typescript';
+import deepEquals = require('fast-deep-equal');
 import {State as AppState} from '~frontend/drivers/appstate';
 import {NetworkSource} from '~frontend/drivers/network';
 import {SSBSource} from '~frontend/drivers/ssb';
@@ -172,6 +173,10 @@ function reevaluateStatus(prev: State): State {
   return prev;
 }
 
+function pickId(peer: PeerKV): string {
+  return peer[0];
+}
+
 export default function model(
   ssbSource: SSBSource,
   networkSource: NetworkSource,
@@ -231,7 +236,15 @@ export default function model(
         const rooms = allPeers.filter(
           ([, data]) => (data.type as any) === 'room',
         );
-        return {...prev, peers, rooms, timestampPeersAndRooms: Date.now()};
+        if (
+          deepEquals(prev.peers.map(pickId), peers.map(pickId)) &&
+          deepEquals(prev.rooms.map(pickId), rooms.map(pickId))
+        ) {
+          // Optimization: nothing changed, so don't emit a new state object
+          return prev;
+        } else {
+          return {...prev, peers, rooms, timestampPeersAndRooms: Date.now()};
+        }
       },
   );
 
@@ -281,7 +294,12 @@ export default function model(
               return peer;
             }
           });
-          return {...prev, peers, timestampPeersAndRooms: Date.now()};
+          if (deepEquals(prev.peers.map(pickId), peers.map(pickId))) {
+            // Optimization: nothing changed, so don't emit a new state object
+            return prev;
+          } else {
+            return {...prev, peers, timestampPeersAndRooms: Date.now()};
+          }
         },
     );
 

diff --git a/src/frontend/screens/central/connections-tab/view.ts b/src/frontend/screens/central/connections-tab/view.ts
@@ -452,8 +452,8 @@ export default function view(state$: Stream<State>) {
         (p) => p[1].state === 'connected',
       );
 
-      return h(View, {style: styles.container}, [
-        h(View, {style: styles.innerContainer}, [
+      return h(View, {key: 'conntab', style: styles.container}, [
+        h(View, {key: 'ic', style: styles.innerContainer}, [
           h(Summary, {connectedPeers}),
           h(Scenario, {status, scenario}),
           h(Recommendations, {

diff --git a/src/frontend/screens/central/index.ts b/src/frontend/screens/central/index.ts
@@ -136,7 +136,8 @@ export function central(sources: Sources): Sinks {
         ? privateTabSinks.fab
         : connectionsTabSinks.fab,
     )
-    .flatten();
+    .flatten()
+    .compose(dropRepeats());
 
   const command$ = navigation(
     state$,

diff --git a/src/frontend/screens/central/model.ts b/src/frontend/screens/central/model.ts
@@ -6,6 +6,7 @@ import xs, {Stream} from 'xstream';
 import {Reducer, Lens} from '@cycle/state';
 import {Animated} from 'react-native';
 import {FeedId, MsgId} from 'ssb-typescript';
+import deepEquals = require('fast-deep-equal');
 import {SSBSource} from '~frontend/drivers/ssb';
 import progressCalculation, {
   State as ProgressState,
@@ -54,14 +55,26 @@ export const publicTabLens: Lens<State, PublicTabState> = {
     const isVisible = parent.currentTab === 'public';
     const {selfFeedId, selfAvatarUrl, canPublishSSB} = parent;
     if (parent.publicTab) {
-      return {
-        ...parent.publicTab,
-        isVisible,
-        selfFeedId,
-        selfAvatarUrl,
-        canPublishSSB,
-      };
+      const prev = parent.publicTab;
+      if (
+        prev.isVisible === isVisible &&
+        prev.selfFeedId === selfFeedId &&
+        prev.selfAvatarUrl === selfAvatarUrl &&
+        prev.canPublishSSB === canPublishSSB
+      ) {
+        // Optimization: nothing changed
+        return prev;
+      } else {
+        return {
+          ...parent.publicTab,
+          isVisible,
+          selfFeedId,
+          selfAvatarUrl,
+          canPublishSSB,
+        };
+      }
     } else {
+      // Initialize the public tab state
       return {
         isVisible,
         selfFeedId,
@@ -79,13 +92,23 @@ export const publicTabLens: Lens<State, PublicTabState> = {
   },
 
   set: (parent: State, child: PublicTabState): State => {
-    return {
-      ...parent,
-      initializedSSB: child.initializedSSB,
-      numOfPublicUpdates: child.numOfUpdates,
-      lastSessionTimestamp: child.lastSessionTimestamp,
-      publicTab: child,
-    };
+    if (
+      parent.initializedSSB === child.initializedSSB &&
+      parent.numOfPublicUpdates === child.numOfUpdates &&
+      parent.lastSessionTimestamp === child.lastSessionTimestamp &&
+      deepEquals(parent.publicTab, child)
+    ) {
+      // Optimization: nothing changed in the child, so don't update the parent
+      return parent;
+    } else {
+      return {
+        ...parent,
+        initializedSSB: child.initializedSSB,
+        numOfPublicUpdates: child.numOfUpdates,
+        lastSessionTimestamp: child.lastSessionTimestamp,
+        publicTab: child,
+      };
+    }
   },
 };
 
@@ -94,8 +117,24 @@ export const privateTabLens: Lens<State, PrivateTabState> = {
     const isVisible = parent.currentTab === 'private';
     const {selfFeedId, selfAvatarUrl} = parent;
     if (parent.privateTab) {
-      return {...parent.privateTab, isVisible, selfFeedId, selfAvatarUrl};
+      const prev = parent.privateTab;
+      if (
+        prev.isVisible === isVisible &&
+        prev.selfFeedId === selfFeedId &&
+        prev.selfAvatarUrl === selfAvatarUrl
+      ) {
+        // Optimization: nothing changed
+        return prev;
+      } else {
+        return {
+          ...parent.privateTab,
+          isVisible,
+          selfFeedId,
+          selfAvatarUrl,
+        };
+      }
     } else {
+      // Initialize the private tab state
       return {
         isVisible,
         selfFeedId,
@@ -109,11 +148,19 @@ export const privateTabLens: Lens<State, PrivateTabState> = {
   },
 
   set: (parent: State, child: PrivateTabState): State => {
-    return {
-      ...parent,
-      numOfPrivateUpdates: child.updates.size,
-      privateTab: child,
-    };
+    if (
+      parent.numOfPrivateUpdates === child.updates.size &&
+      deepEquals(parent.privateTab, child)
+    ) {
+      // Optimization: nothing changed in the child, so don't update the parent
+      return parent;
+    } else {
+      return {
+        ...parent,
+        numOfPrivateUpdates: child.updates.size,
+        privateTab: child,
+      };
+    }
   },
 };
 
@@ -122,8 +169,24 @@ export const activityTabLens: Lens<State, ActivityTabState> = {
     const isVisible = parent.currentTab === 'activity';
     const {selfFeedId, selfAvatarUrl} = parent;
     if (parent.activityTab) {
-      return {...parent.activityTab, isVisible, selfFeedId, selfAvatarUrl};
+      const prev = parent.activityTab;
+      if (
+        prev.isVisible === isVisible &&
+        prev.selfFeedId === selfFeedId &&
+        prev.selfAvatarUrl === selfAvatarUrl
+      ) {
+        // Optimization: nothing changed
+        return prev;
+      } else {
+        return {
+          ...parent.activityTab,
+          isVisible,
+          selfFeedId,
+          selfAvatarUrl,
+        };
+      }
     } else {
+      // Initialize the activity tab state
       return {
         isVisible,
         selfFeedId,
@@ -137,11 +200,19 @@ export const activityTabLens: Lens<State, ActivityTabState> = {
   },
 
   set: (parent: State, child: ActivityTabState): State => {
-    return {
-      ...parent,
-      numOfActivityUpdates: child.numOfUpdates,
-      activityTab: child,
-    };
+    if (
+      parent.numOfActivityUpdates === child.numOfUpdates &&
+      deepEquals(parent.activityTab, child)
+    ) {
+      // Optimization: nothing changed in the child, so don't update the parent
+      return parent;
+    } else {
+      return {
+        ...parent,
+        numOfActivityUpdates: child.numOfUpdates,
+        activityTab: child,
+      };
+    }
   },
 };
 
@@ -150,14 +221,26 @@ export const connectionsTabLens: Lens<State, ConnectionsTabState> = {
     const isVisible = parent.currentTab === 'connections';
     const {selfFeedId, selfAvatarUrl, initializedSSB} = parent;
     if (parent.connectionsTab) {
-      return {
-        ...parent.connectionsTab,
-        isVisible,
-        selfFeedId,
-        selfAvatarUrl,
-        initializedSSB,
-      };
+      const prev = parent.connectionsTab;
+      if (
+        prev.isVisible === isVisible &&
+        prev.selfFeedId === selfFeedId &&
+        prev.selfAvatarUrl === selfAvatarUrl &&
+        prev.initializedSSB === initializedSSB
+      ) {
+        // Optimization: nothing changed
+        return prev;
+      } else {
+        return {
+          ...parent.connectionsTab,
+          isVisible,
+          selfFeedId,
+          selfAvatarUrl,
+          initializedSSB,
+        };
+      }
     } else {
+      // Initialize the connections tab state
       return {
         isVisible,
         selfFeedId,
@@ -180,10 +263,15 @@ export const connectionsTabLens: Lens<State, ConnectionsTabState> = {
   },
 
   set: (parent: State, child: ConnectionsTabState): State => {
-    return {
-      ...parent,
-      connectionsTab: child,
-    };
+    if (deepEquals(parent.connectionsTab, child)) {
+      // Optimization: nothing changed in the child, so don't update the parent
+      return parent;
+    } else {
+      return {
+        ...parent,
+        connectionsTab: child,
+      };
+    }
   },
 };