Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf!: Optimize Filter Initialization with Concurrent Processing #6106

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

qfrank
Copy link
Contributor

@qfrank qfrank commented Nov 18, 2024

Summary

This PR includes several improvements to the messenger's filter initialization and chat preview functionality:

Key Changes:

  • Refactored InitFilters into smaller, more focused functions for better maintainability, and init filters concurrently
  • Moved filter initialization logic from messenger.go to a new file messenger_filter_init.go
  • Added optional community chat filtering to ChatsPreview method
  • Made history archives import asynchronous via startHistoryArchivesImportLoop

The main improvements are:

  1. Better code organization by splitting the large InitFilters function into smaller, focused functions
  2. Enhanced error handling and logging throughout the filter initialization process
  3. Added ability to filter chat previews by community vs non-community chats
  4. Improved performance by making history archives import non-blocking

The changes maintain backward compatibility while providing new filtering capabilities for chat previews.

With this PR, the duration for api StartMessenger has been reduced from 1.71s to 1.02s running with iphone XS Max. Relate mobile issue

Why added ability to filter chat previews by community vs non-community chats?

Let's take a look at chatsPreviewResponse.log, the response is very long which contains around 3.77 million characters. There's lots of chats related to community, when mobile frontend loads the chat preview list, actually it doesn't need the community chats for the chat list data soon, so we can split the long response into 2 parts(one with community chats one with non-community chats). With this solution, we can see the chat preview list almost in real time. It need wait around 2-3 seconds before this solution. But if we want to fix the performance issue totally, I'm wondering if we can use protobuf as data transfer mechanism rather than json 🤔, you can see there's lots of string starts with 0x... like 0x04ffd68fe2bc7b2fceff949fd06c0f7a181ee5df2fc7ce9960c73a08e3c316a196dbae1700855a038bafe5091ab2cee9ac6fa77ed9bffe2053deeb97083d75d59f @igor-sirotin @ilmotta
chatsPreviewResponse.log.zip

@qfrank qfrank self-assigned this Nov 18, 2024
@status-im-auto
Copy link
Member

status-im-auto commented Nov 18, 2024

Jenkins Builds

Click to see older builds (40)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ a507bea #1 2024-11-18 08:08:05 ~4 min macos 📦zip
✔️ a507bea #1 2024-11-18 08:08:30 ~4 min ios 📦zip
✖️ a507bea #1 2024-11-18 08:09:14 ~5 min tests 📄log
✔️ a507bea #1 2024-11-18 08:10:43 ~7 min windows 📦zip
✔️ a507bea #1 2024-11-18 08:10:54 ~7 min linux 📦zip
✔️ a507bea #1 2024-11-18 08:11:40 ~8 min android 📦aar
✔️ a507bea #1 2024-11-18 08:11:41 ~8 min macos 📦zip
✖️ a507bea #1 2024-11-18 08:12:25 ~8 min tests-rpc 📄log
✔️ 478d714 #2 2024-11-18 11:46:52 ~3 min windows 📦zip
✖️ 478d714 #2 2024-11-18 11:46:57 ~4 min tests-rpc 📄log
✔️ 478d714 #2 2024-11-18 11:47:15 ~4 min macos 📦zip
✔️ 478d714 #2 2024-11-18 11:47:30 ~4 min ios 📦zip
✔️ 478d714 #2 2024-11-18 11:48:24 ~5 min linux 📦zip
✔️ 478d714 #2 2024-11-18 11:48:53 ~6 min android 📦aar
✔️ 478d714 #2 2024-11-18 11:50:42 ~7 min macos 📦zip
✔️ 478d714 #2 2024-11-18 12:14:45 ~31 min tests 📄log
✖️ 502d4f3 #3 2024-11-21 08:16:35 ~2 min tests 📄log
✖️ 502d4f3 #3 2024-11-21 08:17:38 ~4 min tests-rpc 📄log
✔️ 502d4f3 #3 2024-11-21 08:18:08 ~4 min macos 📦zip
✔️ 502d4f3 #3 2024-11-21 08:18:10 ~4 min windows 📦zip
✔️ 502d4f3 #3 2024-11-21 08:18:32 ~5 min ios 📦zip
✔️ 502d4f3 #3 2024-11-21 08:19:07 ~5 min linux 📦zip
✔️ 502d4f3 #3 2024-11-21 08:19:51 ~6 min android 📦aar
✔️ 502d4f3 #3 2024-11-21 08:22:48 ~9 min macos 📦zip
✖️ 42023d3 #4 2024-11-21 08:54:37 ~2 min tests 📄log
✔️ 42023d3 #4 2024-11-21 08:56:24 ~4 min windows 📦zip
✔️ 42023d3 #4 2024-11-21 08:56:34 ~4 min macos 📦zip
✔️ 42023d3 #4 2024-11-21 08:56:43 ~4 min ios 📦zip
✔️ 42023d3 #4 2024-11-21 08:56:56 ~4 min tests-rpc 📄log
✔️ 42023d3 #4 2024-11-21 08:57:13 ~5 min android 📦aar
✔️ 42023d3 #4 2024-11-21 08:57:27 ~5 min linux 📦zip
✔️ 42023d3 #4 2024-11-21 09:00:31 ~8 min macos 📦zip
✖️ 5a6e8db #5 2024-11-22 07:40:49 ~3 min tests 📄log
✔️ 5a6e8db #5 2024-11-22 07:41:49 ~4 min windows 📦zip
✖️ 5a6e8db #5 2024-11-22 07:42:01 ~4 min tests-rpc 📄log
✔️ 5a6e8db #5 2024-11-22 07:42:06 ~4 min macos 📦zip
✔️ 5a6e8db #5 2024-11-22 07:42:28 ~4 min ios 📦zip
✔️ 5a6e8db #5 2024-11-22 07:42:52 ~5 min linux 📦zip
✔️ 5a6e8db #5 2024-11-22 07:43:38 ~6 min android 📦aar
✔️ 5a6e8db #5 2024-11-22 07:47:57 ~10 min macos 📦zip
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 7263420 #6 2024-11-22 09:59:48 ~4 min macos 📦zip
✔️ 7263420 #6 2024-11-22 10:00:03 ~4 min windows 📦zip
✖️ 7263420 #6 2024-11-22 10:00:25 ~4 min tests 📄log
✔️ 7263420 #6 2024-11-22 10:00:32 ~4 min linux 📦zip
✔️ 7263420 #6 2024-11-22 10:00:40 ~4 min ios 📦zip
✔️ 7263420 #6 2024-11-22 10:00:53 ~5 min tests-rpc 📄log
✔️ 7263420 #6 2024-11-22 10:02:05 ~6 min android 📦aar
✔️ 7263420 #6 2024-11-22 10:04:19 ~8 min macos 📦zip
✖️ a4a8bb1 #7 2024-11-22 10:41:11 ~3 min tests-rpc 📄log
✔️ a4a8bb1 #7 2024-11-22 10:41:29 ~3 min macos 📦zip
✔️ a4a8bb1 #7 2024-11-22 10:46:56 ~9 min macos 📦zip
✔️ a4a8bb1 #8 2024-11-22 11:11:20 ~5 min android 📦aar
✔️ a4a8bb1 #7 2024-11-22 13:03:51 ~5 min windows 📦zip
✔️ a4a8bb1 #7 2024-11-22 18:25:50 ~4 min ios 📦zip

@qfrank qfrank changed the title perf: Optimize Filter Initialization with Concurrent Processing perf_: Optimize Filter Initialization with Concurrent Processing Nov 18, 2024
Copy link

codecov bot commented Nov 18, 2024

Codecov Report

Attention: Patch coverage is 51.28205% with 114 lines in your changes missing coverage. Please review.

Project coverage is 61.04%. Comparing base (6149fa3) to head (478d714).
Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
protocol/messenger_filter_init.go 51.86% 80 Missing and 23 partials ⚠️
protocol/messenger.go 64.28% 4 Missing and 1 partial ⚠️
protocol/messenger_chats.go 0.00% 4 Missing ⚠️
services/ext/api.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #6106      +/-   ##
===========================================
+ Coverage    60.96%   61.04%   +0.07%     
===========================================
  Files          814      815       +1     
  Lines       109253   109352      +99     
===========================================
+ Hits         66611    66751     +140     
+ Misses       34806    34782      -24     
+ Partials      7836     7819      -17     
Flag Coverage Δ
functional 13.68% <35.89%> (+0.14%) ⬆️
unit 60.26% <51.28%> (+0.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
services/ext/api.go 6.27% <0.00%> (ø)
protocol/messenger_chats.go 34.49% <0.00%> (-0.28%) ⬇️
protocol/messenger.go 63.65% <64.28%> (+1.04%) ⬆️
protocol/messenger_filter_init.go 51.86% <51.86%> (ø)

... and 32 files with indirect coverage changes

protocol/messenger.go Outdated Show resolved Hide resolved
protocol/messenger.go Outdated Show resolved Hide resolved
protocol/messenger.go Show resolved Hide resolved
protocol/messenger_filter_init.go Outdated Show resolved Hide resolved
protocol/messenger_filter_init.go Outdated Show resolved Hide resolved
protocol/messenger.go Show resolved Hide resolved
protocol/messenger_chats.go Outdated Show resolved Hide resolved
services/ext/api.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@igor-sirotin igor-sirotin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work splitting the function 👏
And thanks for the separate commit with moving files, made my review much faster 👍

protocol/messenger.go Outdated Show resolved Hide resolved
protocol/messenger.go Outdated Show resolved Hide resolved
protocol/messenger.go Outdated Show resolved Hide resolved
protocol/messenger.go Outdated Show resolved Hide resolved
protocol/messenger.go Outdated Show resolved Hide resolved
protocol/messenger.go Outdated Show resolved Hide resolved
Copy link
Contributor

@osmaczko osmaczko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks much better now.

services/ext/api.go Outdated Show resolved Hide resolved
protocol/messenger_chats.go Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Nov 22, 2024

Looks like you have BREAKING CHANGES in your PR.
Please make sure to follow 💔How to introduce breaking changes guide:

Check-list

@qfrank qfrank changed the title perf_: Optimize Filter Initialization with Concurrent Processing perf!: Optimize Filter Initialization with Concurrent Processing Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants