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

[Performance] [Tracking] Individual chat messages are slow to render #4022

Closed
marcaaron opened this issue Jul 13, 2021 · 43 comments
Closed

[Performance] [Tracking] Individual chat messages are slow to render #4022

marcaaron opened this issue Jul 13, 2021 · 43 comments
Assignees
Labels
Daily KSv2 Engineering Improvement Item broken or needs improvement. Planning Changes still in the thought process

Comments

@marcaaron
Copy link
Contributor

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  • Tap on a chat

Expected Result:

  • When chat loads the entire visible area is filled with messages instantly

Actual Result:

  • When items first render only a subset of them render and each "batch" of items is slightly delayed and one message renders after the other in a staggered fashion

Workaround:

Wait for the messages to appear awkwardly.

Platform:

All platforms, but the effects are felt differently depending on the platform.

Here's an example of what this looks like on various platforms...

Web:

2021-07-13_12-30-33 (1)

iOS:

2021-07-13_12-58-21 (1)

Version Number:
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL:

View all open jobs on Upwork

@marcaaron marcaaron added the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Jul 13, 2021
@MelvinBot
Copy link

Triggered auto assignment to @lschurr (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

@MelvinBot MelvinBot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Jul 13, 2021
@marcaaron marcaaron added the Daily KSv2 label Jul 13, 2021
@marcaaron marcaaron changed the title Individual chat messages are slow to render [Performance] Individual chat messages are slow to render Jul 13, 2021
@lschurr
Copy link
Contributor

lschurr commented Jul 14, 2021

@marcaaron should this be assigned to you? Or should it be auto-assigned to Eng?

@lschurr lschurr added Engineering Improvement Item broken or needs improvement. labels Jul 14, 2021
@MelvinBot
Copy link

Triggered auto assignment to @Luke9389 (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@lschurr
Copy link
Contributor

lschurr commented Jul 14, 2021

Adding Eng before posting to External..

@lschurr lschurr removed their assignment Jul 14, 2021
@Luke9389
Copy link
Contributor

Ok, I see no reason not to make this external. Let's see what proposals our contributors come up with.

@Luke9389 Luke9389 added the External Added to denote the issue can be worked on by a contributor label Jul 14, 2021
@MelvinBot
Copy link

Triggered auto assignment to @SofiedeVreese (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@marcaaron
Copy link
Contributor Author

@lschurr auto assign to eng is perfect thanks!

@SofiedeVreese
Copy link
Contributor

SofiedeVreese commented Jul 15, 2021

Removing @Luke9389 's assignment.

Posted to Upwork: https://www.upwork.com/jobs/~011974a9e078cd37d9

Adding Exported label now.

@MelvinBot MelvinBot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 15, 2021
@MelvinBot
Copy link

Triggered auto assignment to @deetergp (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@aliabbasmalik8

This comment has been minimized.

@SofiedeVreese
Copy link
Contributor

As I'm out of office tomorrow, I'm going to unassign myself and re-add the External label.

@SofiedeVreese SofiedeVreese removed the External Added to denote the issue can be worked on by a contributor label Jul 15, 2021
@SofiedeVreese SofiedeVreese removed their assignment Jul 15, 2021
@marcaaron marcaaron added the Planning Changes still in the thought process label Jul 23, 2021
@marcaaron marcaaron changed the title [Performance] Individual chat messages are slow to render [Performance] [Tracking] Individual chat messages are slow to render Jul 23, 2021
@marcaaron
Copy link
Contributor Author

@deetergp yes, that's the idea @rdjuric had. I think maybe we can create a new issue for it.

@mallenexpensify
Copy link
Contributor

mallenexpensify commented Jul 23, 2021

Job is closed/removed in Upwork (also checked and Help Wanted label is on this issue. hahahahah, just saw you JUST took them off Marc)

@rdjuric
Copy link
Contributor

rdjuric commented Jul 27, 2021

@deetergp yes, that's the idea @rdjuric had. I think maybe we can create a new issue for it.

@marcaaron I opened a issue for this #4258

@marcaaron
Copy link
Contributor Author

@rdjuric nice! Thanks for that.

@MelvinBot MelvinBot added Overdue and removed Overdue labels Jul 27, 2021
@MelvinBot
Copy link

@marcaaron Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@marcaaron
Copy link
Contributor Author

We're making some pretty interesting progress with this issue and it's starting to become clear that any list item components using withOnyx are going to take a long time to render since they'll always be blocked and "waiting for data" from Onyx. By leveraging the Context API we can avoid creating a new connection and get that time back. Exploring it in this PR.

@MelvinBot MelvinBot removed the Overdue label Aug 2, 2021
@marcaaron
Copy link
Contributor Author

So, I think chat switching is starting to approach something resembling an acceptable amount of time. Check out these latest videos which are release builds off main + the changes in the last PR I linked above...

iOS

iOS.Chat.Switch.-.after.improvements.mov

Android

Android.Chat.Switch.-.After.Improvements.mp4

Android looks slightly slower than iOS still (and is a little janky or something at one part) but whatever we are doing is working. 🍻

@quinthar
Copy link
Contributor

quinthar commented Aug 3, 2021 via email

@MelvinBot
Copy link

@marcaaron Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@MelvinBot
Copy link

@marcaaron 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

@marcaaron
Copy link
Contributor Author

Ok found a couple more improvements in these two PRs:

#4573
#4538

I think once they are merged we can close this issue.

@MelvinBot
Copy link

@marcaaron Huh... This is 4 days overdue. Who can take care of this?

@MelvinBot
Copy link

@marcaaron Still overdue 6 days?! Let's take care of this!

@marcaaron
Copy link
Contributor Author

Going to close this for now as the last of the changes are on staging. I'm not sure what else we can do to improve the individual loading of chat messages and they seem to load pretty fast for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Daily KSv2 Engineering Improvement Item broken or needs improvement. Planning Changes still in the thought process
Projects
None yet
Development

No branches or pull requests