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

Optimize rerendering of recent conversations for single message events. #23638

Open
laurynmm opened this issue Nov 21, 2022 · 12 comments
Open

Optimize rerendering of recent conversations for single message events. #23638

laurynmm opened this issue Nov 21, 2022 · 12 comments
Labels
area: recent-conversations Issues related to "Recent conversations" view. help wanted

Comments

@laurynmm
Copy link
Collaborator

Currently, when new messages are received (either via the events API or fetching messages), the Recent conversations view will do a complete_rerender if any of the processed messages has data that has been updated/added to the view.

Instead of completely rerendering when processing a single message (most likely from the events API), it would preferable to optimize so that we only update the conversation that was updated for the new message data when/if possible.

Follow-up issue from #23603 and #23617.

@zulipbot add "area: recent-conversations"

@zulipbot zulipbot added the area: recent-conversations Issues related to "Recent conversations" view. label Nov 21, 2022
@laurynmm
Copy link
Collaborator Author

@zulipbot add "help wanted"

@cledi01
Copy link
Collaborator

cledi01 commented Nov 26, 2022

Hi @laurynmm , this issue looks really interesting so I started digging around :). So currently, as far as my understanding goes when the messages are being fetched for the first time to complete-rerender being called, this is the callstack:

image

and this is the callstack when a message comes in from the events api:
image

And it seems like complete_rerender is being called indiscriminately in the process_messages function. I think at the "insert_new_messages" function, I'm thinking a simple conditional and a new function "update_message" in recent_topics_ui.js can solve the problem.

Does this approach sound reasonabe to you? I would really appreciate the feedback.

@laurynmm
Copy link
Collaborator Author

@cledi01 - I think it'd be good to look at the code paths that call recent_topics_ui.inplace_rerender as well since those are examples of cases where we're already not doing a complete rerender of the view. See this comment from one of the pull requests noted above.

@dartaylor8
Copy link

Hi, I am a student at Carnegie Mellon working on a group project for my Foundations of Software Engineering class.

Is it ok if my group helps out with this issue for our project?

@ilaiyengar
Copy link
Collaborator

@zulipbot claim

@alya
Copy link
Contributor

alya commented Nov 29, 2022

@dartaylor8 We don't have any special rules for class projects. Your group should follow the general guidelines for all contributors. https://zulip.readthedocs.io/en/latest/overview/contributing.html

Have fun!

@ilaiyengar
Copy link
Collaborator

@laurynmm I am working on a solution and was trying to look into inplace_rerender. Do you happen to know what exactly this function's intended purpose is? Thanks!

@laurynmm
Copy link
Collaborator Author

laurynmm commented Dec 5, 2022

@ilaiyengar - I would look at the places that inplace_rerender is used in the static/js directory. So, doing a git-grep for that would give you the difference places to look for when / why / how that function is being called: git grep 'inplace_rerender' static/js. If you're not super familiar with git-grep, here's a blog post I wrote about it when I was learning to use it last year.

@ilaiyengar
Copy link
Collaborator

Thanks so much! I will look into this.

@zulipbot
Copy link
Member

zulipbot commented Dec 16, 2022

@ilaiyengar You have been unassigned from this issue because you have not made any updates for over 14 days. Please feel free to reclaim the issue if you decide to pick up again. Thanks!

@Ujjawal3
Copy link
Collaborator

@zulipbot claim.

@zulipbot
Copy link
Member

zulipbot commented Mar 30, 2023

@Ujjawal3 You have been unassigned from this issue because you have not made any updates for over 14 days. Please feel free to reclaim the issue if you decide to pick up again. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: recent-conversations Issues related to "Recent conversations" view. help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants