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

fix_: message history loading took too much time #21411

Merged
merged 1 commit into from
Oct 18, 2024

Conversation

qfrank
Copy link
Contributor

@qfrank qfrank commented Oct 10, 2024

For details pls see description in related status-go PR

fixes #21358

@status-im-auto
Copy link
Member

status-im-auto commented Oct 10, 2024

Jenkins Builds

Click to see older builds (16)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 953d6d6 #1 2024-10-10 01:47:31 ~6 min tests 📄log
✔️ 953d6d6 #1 2024-10-10 01:51:00 ~10 min ios 📱ipa 📲
✔️ 953d6d6 #1 2024-10-10 01:51:06 ~10 min android-e2e 🤖apk 📲
✔️ 953d6d6 #1 2024-10-10 01:52:25 ~11 min android 🤖apk 📲
✔️ 5261fec #2 2024-10-10 09:00:15 ~6 min tests 📄log
✔️ 5261fec #2 2024-10-10 09:03:48 ~10 min android-e2e 🤖apk 📲
✔️ 5261fec #2 2024-10-10 09:05:17 ~11 min android 🤖apk 📲
✔️ 5261fec #2 2024-10-10 09:05:57 ~12 min ios 📱ipa 📲
✔️ 4aeeef6 #3 2024-10-11 03:33:45 ~4 min tests 📄log
✔️ 4aeeef6 #3 2024-10-11 03:39:17 ~10 min ios 📱ipa 📲
✔️ 4aeeef6 #3 2024-10-11 03:40:39 ~11 min android-e2e 🤖apk 📲
✔️ 4aeeef6 #3 2024-10-11 03:42:00 ~12 min android 🤖apk 📲
✔️ de57142 #4 2024-10-14 06:39:59 ~6 min tests 📄log
✔️ de57142 #4 2024-10-14 06:43:49 ~10 min android-e2e 🤖apk 📲
✔️ de57142 #4 2024-10-14 06:44:04 ~10 min ios 📱ipa 📲
✔️ de57142 #4 2024-10-14 06:45:13 ~11 min android 🤖apk 📲
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 4f41b46 #5 2024-10-16 10:13:08 ~7 min tests 📄log
✔️ 4f41b46 #5 2024-10-16 10:13:36 ~7 min android-e2e 🤖apk 📲
✔️ 4f41b46 #5 2024-10-16 10:15:30 ~9 min android 🤖apk 📲
✔️ 4f41b46 #5 2024-10-16 10:16:36 ~10 min ios 📱ipa 📲
✔️ d53c1f5 #6 2024-10-18 02:41:07 ~5 min tests 📄log
✔️ d53c1f5 #6 2024-10-18 02:42:19 ~6 min android-e2e 🤖apk 📲
✔️ d53c1f5 #6 2024-10-18 02:45:20 ~9 min ios 📱ipa 📲
✔️ d53c1f5 #6 2024-10-18 02:45:50 ~10 min android 🤖apk 📲

@qfrank qfrank requested a review from seanstrom October 10, 2024 06:25
@qfrank qfrank marked this pull request as ready for review October 10, 2024 06:27
@qfrank qfrank force-pushed the fix/message_history_loading branch from 953d6d6 to 5261fec Compare October 10, 2024 08:53
@qfrank qfrank force-pushed the fix/message_history_loading branch 3 times, most recently from de57142 to 4f41b46 Compare October 16, 2024 10:05
@mariia-skrypnyk mariia-skrypnyk self-assigned this Oct 16, 2024
@mariia-skrypnyk
Copy link

mariia-skrypnyk commented Oct 17, 2024

Hey @qfrank !

Thanks for your PR.
Loading time is reduced to max 20 sec till all messages appear for receiver.

I have 2 questions to discuss. The second one can be a separate issue I guess.

  1. Is it expected that the Receiver sees messages loading chaotically? For example, the last messages may load first, then the first ones, and then the ones in between? I have tested this multiple times, and it is not a one-time occurrence when there are more than 10 messages.
  2. When I tested PR on one device (Receiver) and used another device with nightly build (Sender) I observed that several messages got lost for a Receiver. They didn't come even after 30 min. I assume this is not a scope of a current PR and can report a separate issue. When I tested PR-PR devices everything was fine with no lost messages.

If both my questions are not in scope of current PR then PR can be merged.

@status-im-auto
Copy link
Member

75% of end-end tests have passed

Total executed tests: 8
Failed tests: 2
Expected to fail tests: 0
Passed tests: 6
IDs of failed tests: 740490,702843 

Failed tests (2)

Click to expand
  • Rerun failed tests

  • Class TestWalletOneDevice:

    1. test_wallet_balance_mainnet, id: 740490

    Device 1: Find `Button` by `accessibility id`: `network-dropdown`
    Device 1: Tap on found: Button

    critical/test_wallet.py:249: in test_wallet_balance_mainnet
        self.errors.verify_no_errors()
    base_test_case.py:192: in verify_no_errors
        pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
     For the Ether the wrong value 0.0051 is shown, expected 0.0052 in total
    E    For the Uniswap the wrong value 0.127 is shown, expected 0.627 in total
    E    For the Ether the wrong value 0.0 is shown, expected 0.0001 on Arbitrum
    E    For the Uniswap the wrong value 0.0 is shown, expected 0.5 on Arbitrum
    



    Class TestCommunityMultipleDeviceMerged:

    1. test_community_message_edit, id: 702843

    Device 2: Find Text by xpath: //android.view.ViewGroup[@content-desc='chat-item']//android.widget.TextView[contains(@text,'https://status.app/c/')]
    Device 2: Wait for element Button for max 120s and click when it is available

    Test setup failed: critical/chats/test_public_chat_browsing.py:350: in prepare_devices
        self.community_2.join_community()
    ../views/chat_view.py:420: in join_community
        self.join_button.wait_and_click(120)
    ../views/base_element.py:100: in wait_and_click
        self.wait_for_visibility_of_element(sec)
    ../views/base_element.py:147: in wait_for_visibility_of_element
        raise TimeoutException(
     Device 2: Button by accessibility id:`show-request-to-join-screen-button` is not found on the screen after wait_for_visibility_of_element
    



    Device sessions

    Passed tests (6)

    Click to expand

    Class TestWalletMultipleDevice:

    1. test_wallet_send_asset_from_drawer, id: 727230
    2. test_wallet_send_eth, id: 727229

    Class TestWalletOneDevice:

    1. test_wallet_add_remove_regular_account, id: 727231

    Class TestOneToOneChatMultipleSharedDevicesNewUi:

    1. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745
    Device sessions

    Class TestCommunityOneDeviceMerged:

    1. test_community_copy_and_paste_message_in_chat_input, id: 702742
    Device sessions

    2. test_restore_multiaccount_with_waku_backup_remove_switch, id: 703133
    Device sessions

    @qfrank
    Copy link
    Contributor Author

    qfrank commented Oct 17, 2024

    Hey @qfrank !

    Thanks for your PR. Loading time is reduced to max 20 sec till all messages appear for receiver.

    I have 2 questions to discuss. The second one can be a separate issue I guess.

    1. Is it expected that the Receiver sees messages loading chaotically? For example, the last messages may load first, then the first ones, and then the ones in between? I have tested this multiple times, and it is not a one-time occurrence when there are more than 10 messages.
    2. When I tested PR on one device (Receiver) and used another device with nightly build (Sender) I observed that several messages got lost for a Receiver. They didn't come even after 30 min. I assume this is not a scope of a current PR and can report a separate issue. When I tested PR-PR devices everything was fine with no lost messages.

    If both my questions are not in scope of current PR then PR can be merged.

    @mariia-skrypnyk Thank you for testing!
    I also have serveral questions for you :)

    • Could you tell me how long it took before this PR?
    • How long it take until you see the chat list appear after login?
    • Could you share the log so that I can continue optimising it in another PR as 20 seconds still sounds not good but maybe in another PR

    Is it expected that the Receiver sees messages loading chaotically? For example, the last messages may load first, then the first ones, and then the ones in between? I have tested this multiple times, and it is not a one-time occurrence when there are more than 10 messages.

    From a normal user view, this should definitely be fixed. I doubt if it's relate to this PR, can you reproduce it without this PR?

    When I tested PR on one device (Receiver) and used another device with nightly build (Sender) I observed that several messages got lost for a Receiver. They didn't come even after 30 min. I assume this is not a scope of a current PR and can report a separate issue. When I tested PR-PR devices everything was fine with no lost messages.

    Let's create a separate issue for this and attach the logs for sender and receiver, both debug level pls 🙏

    @churik
    Copy link
    Member

    churik commented Oct 17, 2024

    Confirmed by @ilmotta that waku has no change in this Pr, so shouldn't make a difference;

    Speaking of:

    Is it expected that the Receiver sees messages loading chaotically?

    It was always like this, just eventually it should be in the same order that it was sent (so the sequence in the chat view after all messages are loaded will be the same eventually).

    So basically we should check that that performance is better in this PR in terms of communiy /chat loading

    @mariia-skrypnyk
    Copy link

    mariia-skrypnyk commented Oct 17, 2024

    Hey @qfrank !

    I also have serveral questions for you :)

    Always welcome!)

    1) Could you tell me how long it took before this PR?
    2) How long it take until you see the chat list appear after login?

    will combine these 2 questions because for me chat appeared after 2 min of waiting and contained all uploaded messages (develop). I've tried several times to trick this process by scaning Sender's QR to get into the chat quicker (wanted to observe in what order messages are loading) but no chance to have chat in the list of chats less then 2 min of waiting

    3) From a normal user view, this should definitely be fixed. I doubt if it's relate to this PR, can you reproduce it without this PR?

    yes, I cought it on develop too

    4) Let's create a separate issue for this and attach the logs for sender and receiver, both debug level pls 🙏

    ok, will do a separate issue

    Thanks @churik!

    It was always like this

    I assume that it's not a correct behaviour and I will create an issue on this.
    Greate if we can fix it.

    performance is better in this PR in terms of communiy /chat loading

    definatelly is!

    @qfrank you are free to merge it 🙌

    @qfrank
    Copy link
    Contributor Author

    qfrank commented Oct 18, 2024

    Hey @qfrank !

    I also have serveral questions for you :)

    Always welcome!)

    1) Could you tell me how long it took before this PR? 2) How long it take until you see the chat list appear after login?

    will combine these 2 questions because for me chat appeared after 2 min of waiting and contained all uploaded messages (develop). I've tried several times to trick this process by scaning Sender's QR to get into the chat quicker (wanted to observe in what order messages are loading) but no chance to have chat in the list of chats less then 2 min of waiting

    3) From a normal user view, this should definitely be fixed. I doubt if it's relate to this PR, can you reproduce it without this PR?

    yes, I cought it on develop too

    4) Let's create a separate issue for this and attach the logs for sender and receiver, both debug level pls 🙏

    ok, will do a separate issue

    Thanks @churik!

    It was always like this

    I assume that it's not a correct behaviour and I will create an issue on this. Greate if we can fix it.

    performance is better in this PR in terms of communiy /chat loading

    definatelly is!

    @qfrank you are free to merge it 🙌

    what about the logs ... @mariia-skrypnyk

    @qfrank qfrank force-pushed the fix/message_history_loading branch from 4f41b46 to d53c1f5 Compare October 18, 2024 02:35
    @qfrank qfrank merged commit dd0ba49 into develop Oct 18, 2024
    5 checks passed
    @qfrank qfrank deleted the fix/message_history_loading branch October 18, 2024 02:51
    @mariia-skrypnyk
    Copy link

    Hi @qfrank !

    Thanks for merge 🙌.
    I've created separate issue for investigation of the ability to more optimize perfomance of messages reload.
    You can find it here #21456.

    @qfrank
    Copy link
    Contributor Author

    qfrank commented Oct 18, 2024

    Thanks for creating the followup issue, but the attached log didn't contain requests.log since it's not PR build , could you try this PR again and attach the new logs? @mariia-skrypnyk

    @mariia-skrypnyk
    Copy link

    Thanks for catching it @qfrank .

    I've changed logs there.
    BTW loading this time took 7 sec but it was the first time it took so fast.
    Another my testing attempts took aprox 18-20 sec.
    Let me know if anything else is needed.
    Also I suppose I need to change my logs here #21460 ?
    Or develop logs appropriate in this case?

    @qfrank
    Copy link
    Contributor Author

    qfrank commented Oct 18, 2024

    Also I suppose I need to change my logs here #21460 ?

    No need for 21460 ❤️

    @pavloburykh
    Copy link
    Contributor

    pavloburykh commented Oct 18, 2024

    Hey @mariia-skrypnyk @qfrank

    I have also checked performance from my side. Below is comparison between PR and nightly Android builds.

    PR - took 8 seconds to load chats and unread messages
    Nightly - took 17 seconds to load chats and unread messages

    So it seems like we really see improvement in PR. But at the same time we still have space for additional improvements. @qfrank what do you think, does it makes sense to keep #21358 open (or we can open a new similar issue with updated statistics)?

    cc @churik

    On the video below PR screen is at the right and nightly is at the left part.

    pr_nightly_performance_comparison.mp4

    @qfrank
    Copy link
    Contributor Author

    qfrank commented Oct 21, 2024

    does it makes sense to keep #21358 open (or we can open a new similar issue with updated statistics)?

    There is a followup issue, I'll continue to improve it @pavloburykh

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    Archived in project
    Development

    Successfully merging this pull request may close these issues.

    Chats and message history loading after login takes too much time (30-40 seconds)
    6 participants