Skip to content

Commit d70b6cd

Browse files
committed
ux: bug fix: memory leak in the connections tab
1 parent d5095ba commit d70b6cd

8 files changed

Lines changed: 242 additions & 126 deletions

File tree

package-lock.json

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@
9696
"cycle-native-toastandroid": "1.1.0",
9797
"electron-context-menu": "3.1.2",
9898
"electron-ssb-client": "2.0.0",
99+
"fast-deep-equal": "3.1.3",
99100
"fix-webm-duration": "1.0.4",
100101
"hermes-engine": "0.9.0",
101102
"i18n-js": "3.8.0",

src/frontend/screens/central/connections-tab/model.ts

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import xs, {Stream} from 'xstream';
66
import sample from 'xstream-sample';
77
import concat from 'xstream/extra/concat';
88
import {FeedId} from 'ssb-typescript';
9+
import deepEquals = require('fast-deep-equal');
910
import {State as AppState} from '~frontend/drivers/appstate';
1011
import {NetworkSource} from '~frontend/drivers/network';
1112
import {SSBSource} from '~frontend/drivers/ssb';
@@ -172,6 +173,10 @@ function reevaluateStatus(prev: State): State {
172173
return prev;
173174
}
174175

176+
function pickId(peer: PeerKV): string {
177+
return peer[0];
178+
}
179+
175180
export default function model(
176181
ssbSource: SSBSource,
177182
networkSource: NetworkSource,
@@ -231,7 +236,15 @@ export default function model(
231236
const rooms = allPeers.filter(
232237
([, data]) => (data.type as any) === 'room',
233238
);
234-
return {...prev, peers, rooms, timestampPeersAndRooms: Date.now()};
239+
if (
240+
deepEquals(prev.peers.map(pickId), peers.map(pickId)) &&
241+
deepEquals(prev.rooms.map(pickId), rooms.map(pickId))
242+
) {
243+
// Optimization: nothing changed, so don't emit a new state object
244+
return prev;
245+
} else {
246+
return {...prev, peers, rooms, timestampPeersAndRooms: Date.now()};
247+
}
235248
},
236249
);
237250

@@ -281,7 +294,12 @@ export default function model(
281294
return peer;
282295
}
283296
});
284-
return {...prev, peers, timestampPeersAndRooms: Date.now()};
297+
if (deepEquals(prev.peers.map(pickId), peers.map(pickId))) {
298+
// Optimization: nothing changed, so don't emit a new state object
299+
return prev;
300+
} else {
301+
return {...prev, peers, timestampPeersAndRooms: Date.now()};
302+
}
285303
},
286304
);
287305

src/frontend/screens/central/connections-tab/view.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -452,8 +452,8 @@ export default function view(state$: Stream<State>) {
452452
(p) => p[1].state === 'connected',
453453
);
454454

455-
return h(View, {style: styles.container}, [
456-
h(View, {style: styles.innerContainer}, [
455+
return h(View, {key: 'conntab', style: styles.container}, [
456+
h(View, {key: 'ic', style: styles.innerContainer}, [
457457
h(Summary, {connectedPeers}),
458458
h(Scenario, {status, scenario}),
459459
h(Recommendations, {

src/frontend/screens/central/index.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,8 @@ export function central(sources: Sources): Sinks {
136136
? privateTabSinks.fab
137137
: connectionsTabSinks.fab,
138138
)
139-
.flatten();
139+
.flatten()
140+
.compose(dropRepeats());
140141

141142
const command$ = navigation(
142143
state$,

src/frontend/screens/central/model.ts

Lines changed: 125 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import xs, {Stream} from 'xstream';
66
import {Reducer, Lens} from '@cycle/state';
77
import {Animated} from 'react-native';
88
import {FeedId, MsgId} from 'ssb-typescript';
9+
import deepEquals = require('fast-deep-equal');
910
import {SSBSource} from '~frontend/drivers/ssb';
1011
import progressCalculation, {
1112
State as ProgressState,
@@ -54,14 +55,26 @@ export const publicTabLens: Lens<State, PublicTabState> = {
5455
const isVisible = parent.currentTab === 'public';
5556
const {selfFeedId, selfAvatarUrl, canPublishSSB} = parent;
5657
if (parent.publicTab) {
57-
return {
58-
...parent.publicTab,
59-
isVisible,
60-
selfFeedId,
61-
selfAvatarUrl,
62-
canPublishSSB,
63-
};
58+
const prev = parent.publicTab;
59+
if (
60+
prev.isVisible === isVisible &&
61+
prev.selfFeedId === selfFeedId &&
62+
prev.selfAvatarUrl === selfAvatarUrl &&
63+
prev.canPublishSSB === canPublishSSB
64+
) {
65+
// Optimization: nothing changed
66+
return prev;
67+
} else {
68+
return {
69+
...parent.publicTab,
70+
isVisible,
71+
selfFeedId,
72+
selfAvatarUrl,
73+
canPublishSSB,
74+
};
75+
}
6476
} else {
77+
// Initialize the public tab state
6578
return {
6679
isVisible,
6780
selfFeedId,
@@ -79,13 +92,23 @@ export const publicTabLens: Lens<State, PublicTabState> = {
7992
},
8093

8194
set: (parent: State, child: PublicTabState): State => {
82-
return {
83-
...parent,
84-
initializedSSB: child.initializedSSB,
85-
numOfPublicUpdates: child.numOfUpdates,
86-
lastSessionTimestamp: child.lastSessionTimestamp,
87-
publicTab: child,
88-
};
95+
if (
96+
parent.initializedSSB === child.initializedSSB &&
97+
parent.numOfPublicUpdates === child.numOfUpdates &&
98+
parent.lastSessionTimestamp === child.lastSessionTimestamp &&
99+
deepEquals(parent.publicTab, child)
100+
) {
101+
// Optimization: nothing changed in the child, so don't update the parent
102+
return parent;
103+
} else {
104+
return {
105+
...parent,
106+
initializedSSB: child.initializedSSB,
107+
numOfPublicUpdates: child.numOfUpdates,
108+
lastSessionTimestamp: child.lastSessionTimestamp,
109+
publicTab: child,
110+
};
111+
}
89112
},
90113
};
91114

@@ -94,8 +117,24 @@ export const privateTabLens: Lens<State, PrivateTabState> = {
94117
const isVisible = parent.currentTab === 'private';
95118
const {selfFeedId, selfAvatarUrl} = parent;
96119
if (parent.privateTab) {
97-
return {...parent.privateTab, isVisible, selfFeedId, selfAvatarUrl};
120+
const prev = parent.privateTab;
121+
if (
122+
prev.isVisible === isVisible &&
123+
prev.selfFeedId === selfFeedId &&
124+
prev.selfAvatarUrl === selfAvatarUrl
125+
) {
126+
// Optimization: nothing changed
127+
return prev;
128+
} else {
129+
return {
130+
...parent.privateTab,
131+
isVisible,
132+
selfFeedId,
133+
selfAvatarUrl,
134+
};
135+
}
98136
} else {
137+
// Initialize the private tab state
99138
return {
100139
isVisible,
101140
selfFeedId,
@@ -109,11 +148,19 @@ export const privateTabLens: Lens<State, PrivateTabState> = {
109148
},
110149

111150
set: (parent: State, child: PrivateTabState): State => {
112-
return {
113-
...parent,
114-
numOfPrivateUpdates: child.updates.size,
115-
privateTab: child,
116-
};
151+
if (
152+
parent.numOfPrivateUpdates === child.updates.size &&
153+
deepEquals(parent.privateTab, child)
154+
) {
155+
// Optimization: nothing changed in the child, so don't update the parent
156+
return parent;
157+
} else {
158+
return {
159+
...parent,
160+
numOfPrivateUpdates: child.updates.size,
161+
privateTab: child,
162+
};
163+
}
117164
},
118165
};
119166

@@ -122,8 +169,24 @@ export const activityTabLens: Lens<State, ActivityTabState> = {
122169
const isVisible = parent.currentTab === 'activity';
123170
const {selfFeedId, selfAvatarUrl} = parent;
124171
if (parent.activityTab) {
125-
return {...parent.activityTab, isVisible, selfFeedId, selfAvatarUrl};
172+
const prev = parent.activityTab;
173+
if (
174+
prev.isVisible === isVisible &&
175+
prev.selfFeedId === selfFeedId &&
176+
prev.selfAvatarUrl === selfAvatarUrl
177+
) {
178+
// Optimization: nothing changed
179+
return prev;
180+
} else {
181+
return {
182+
...parent.activityTab,
183+
isVisible,
184+
selfFeedId,
185+
selfAvatarUrl,
186+
};
187+
}
126188
} else {
189+
// Initialize the activity tab state
127190
return {
128191
isVisible,
129192
selfFeedId,
@@ -137,11 +200,19 @@ export const activityTabLens: Lens<State, ActivityTabState> = {
137200
},
138201

139202
set: (parent: State, child: ActivityTabState): State => {
140-
return {
141-
...parent,
142-
numOfActivityUpdates: child.numOfUpdates,
143-
activityTab: child,
144-
};
203+
if (
204+
parent.numOfActivityUpdates === child.numOfUpdates &&
205+
deepEquals(parent.activityTab, child)
206+
) {
207+
// Optimization: nothing changed in the child, so don't update the parent
208+
return parent;
209+
} else {
210+
return {
211+
...parent,
212+
numOfActivityUpdates: child.numOfUpdates,
213+
activityTab: child,
214+
};
215+
}
145216
},
146217
};
147218

@@ -150,14 +221,26 @@ export const connectionsTabLens: Lens<State, ConnectionsTabState> = {
150221
const isVisible = parent.currentTab === 'connections';
151222
const {selfFeedId, selfAvatarUrl, initializedSSB} = parent;
152223
if (parent.connectionsTab) {
153-
return {
154-
...parent.connectionsTab,
155-
isVisible,
156-
selfFeedId,
157-
selfAvatarUrl,
158-
initializedSSB,
159-
};
224+
const prev = parent.connectionsTab;
225+
if (
226+
prev.isVisible === isVisible &&
227+
prev.selfFeedId === selfFeedId &&
228+
prev.selfAvatarUrl === selfAvatarUrl &&
229+
prev.initializedSSB === initializedSSB
230+
) {
231+
// Optimization: nothing changed
232+
return prev;
233+
} else {
234+
return {
235+
...parent.connectionsTab,
236+
isVisible,
237+
selfFeedId,
238+
selfAvatarUrl,
239+
initializedSSB,
240+
};
241+
}
160242
} else {
243+
// Initialize the connections tab state
161244
return {
162245
isVisible,
163246
selfFeedId,
@@ -180,10 +263,15 @@ export const connectionsTabLens: Lens<State, ConnectionsTabState> = {
180263
},
181264

182265
set: (parent: State, child: ConnectionsTabState): State => {
183-
return {
184-
...parent,
185-
connectionsTab: child,
186-
};
266+
if (deepEquals(parent.connectionsTab, child)) {
267+
// Optimization: nothing changed in the child, so don't update the parent
268+
return parent;
269+
} else {
270+
return {
271+
...parent,
272+
connectionsTab: child,
273+
};
274+
}
187275
},
188276
};
189277

0 commit comments

Comments
 (0)