This PR fixes some simple (but tough to track down) memory leaks in the Sphinx Mac app that were causing memory usage to increase significantly every time the user switched between chats, to the point where performance was severely impacted, especially on Intel platform.
To verify the fix, first build and run the app from the test-flight branch and switch between some chats while observing the Memory Use in the Xcode Debug Navigator. Now checkout this PR's branch, build and run and switch between the same chats in the same order. Notice that overall memory usage has decreased significantly, and the usage delta when switching between chats is much more reasonable.
Additional details
I first generated some fake Contacts and Chats for test purposes. I created three contacts with associated Chats: "Zero Chats", "1000 Text Only Messages", "1000 Image Attachments". Using this data I recorded some baseline data, looking at overall memory usage as well as some specific object counts, e.g.
Exploring the Xcode Memory graph we see multiple instances of classes like NewChatViewController are being kept alive even after there are no useful references to them.
We can see the issue in realtime using the Allocations tool in Instruments:
Using this tool, with Record reference counting turned on we can eventually find the needle in the haystack, e.g. here we see a reference count increasing when we call makeCellProvider and we notice this is never balanced out with a release:
That leads us to see that we need to fix the retain cycle in the closure in makeCellProvider. Once we do that (and fix the other issues in this PR), we can rerun the app and observe in Instruments that when we switch chats, the current chat objects are deallocated as expected:
Now when we rerun the tests detailed above switching between multiple chats, we record the following:
This data is from a simple test run of only switching between chats 3 times. The improvement to the memory consumption and performance of the app over a longer period of hours or days or use will of course be much more significant.
Possible next steps/future work
Build automation for running perf tests more easily to help with future improvements and identify perf regressions.
Modify custom view structure to fix memory leaks in NIB loader code
Continue to hunt down major memory leaks, focus on Tribes, web content, etc. Agree acceptable levels
This PR fixes some simple (but tough to track down) memory leaks in the Sphinx Mac app that were causing memory usage to increase significantly every time the user switched between chats, to the point where performance was severely impacted, especially on Intel platform.
Fixes https://github.com/stakwork/sphinx-mac/issues/169 and https://github.com/stakwork/sphinx-mac/issues/160
To verify the fix, first build and run the app from the
test-flightbranch and switch between some chats while observing the Memory Use in the Xcode Debug Navigator. Now checkout this PR's branch, build and run and switch between the same chats in the same order. Notice that overall memory usage has decreased significantly, and the usage delta when switching between chats is much more reasonable.Additional details
I first generated some fake Contacts and Chats for test purposes. I created three contacts with associated Chats: "Zero Chats", "1000 Text Only Messages", "1000 Image Attachments". Using this data I recorded some baseline data, looking at overall memory usage as well as some specific object counts, e.g.
Test:
Baseline (
test-flightbranch @eed709f) Memory: 214MB (192MB on Intel MacBook)AspectFillNSImageViewcount: 475NewOnlyTextMessageCollectionViewitemcount: 90VerticallyCenteredButtonCellcount: 236 ...Exploring the Xcode Memory graph we see multiple instances of classes like
NewChatViewControllerare being kept alive even after there are no useful references to them.We can see the issue in realtime using the Allocations tool in Instruments:
Using this tool, with Record reference counting turned on we can eventually find the needle in the haystack, e.g. here we see a reference count increasing when we call makeCellProvider and we notice this is never balanced out with a release:
That leads us to see that we need to fix the retain cycle in the closure in
makeCellProvider. Once we do that (and fix the other issues in this PR), we can rerun the app and observe in Instruments that when we switch chats, the current chat objects are deallocated as expected:Now when we rerun the tests detailed above switching between multiple chats, we record the following:
Final (
rodhan/perf-improvementsbranch @8c01628) Memory: 120MB (was 214MB, Δ -94MB) (100MB on Intel MacBook (was 192MB, Δ -92MB))AspectFillNSImageViewcount: 201 (was 475, Δ -274)NewOnlyTextMessageCollectionViewitemcount: 30 (was 90, Δ -60)VerticallyCenteredButtonCellcount: 122 (was 236, Δ -114) ...This data is from a simple test run of only switching between chats 3 times. The improvement to the memory consumption and performance of the app over a longer period of hours or days or use will of course be much more significant.
Possible next steps/future work