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 conversations update on Frontend #9832

Merged
merged 1 commit into from
Jun 22, 2023

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Jun 21, 2023

☑️ Resolves

🖼️ Screenshots

No visual changes

🚧 Tasks

  • Add new action addConversationIfChange to add new conversation only if it was moidified according to lastActivity and modifiedSince
  • Add new action postAddConversation for common post-action of adding a conversation
  • Use new addConversationIfChange in fetchConversations action
  • Fix old tests
  • Add new test for this case

🏁 Checklist

Alternative and additional solutions

@ShGKme ShGKme force-pushed the fix/9630/optimize-conversations-update branch 2 times, most recently from 5ddf7d7 to eda959c Compare June 22, 2023 06:51
@ShGKme ShGKme self-assigned this Jun 22, 2023
@ShGKme ShGKme added this to the 💚 Next Patch (27) milestone Jun 22, 2023
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
@ShGKme ShGKme force-pushed the fix/9630/optimize-conversations-update branch from eda959c to d8c10b6 Compare June 22, 2023 07:09
@ShGKme ShGKme marked this pull request as ready for review June 22, 2023 07:16
@ShGKme ShGKme requested review from nickvergessen, Antreesy and marcoambrosini and removed request for nickvergessen June 22, 2023 07:16
@ShGKme ShGKme changed the title WIP Optimize conversations update Optimize conversations update on Frontend Jun 22, 2023
@ShGKme
Copy link
Contributor Author

ShGKme commented Jun 22, 2023

/backport to stable27

Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

🤞🏼

Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

Looks good from the code perspective (but still lagging for me because of internal signalling)

addConversationIfChanged(context, { conversation, modifiedSince }) {
if (conversation.lastActivity >= modifiedSince) {
context.commit('addConversation', conversation)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
}
} else {
// Smart update
}

Don't we want to apply smartUpdate here to cover rest cases (full re-fetch at least)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we use the smartUpdate approach in else, then we do not need if branch, as smartUpdate covers all possible updates anyway

@ShGKme
Copy link
Contributor Author

ShGKme commented Jun 22, 2023

but still lagging for me because of internal signalling

To test on internal you can add:

this.roomListModifiedBefore = ~~(Date.now() / 1000)

after this if

if (loadState('spreed', 'signaling_mode') !== 'internal') {
if (response?.headers && response.headers['x-nextcloud-talk-modified-before']) {
this.roomListModifiedBefore = response.headers['x-nextcloud-talk-modified-before']
}
}
this.initialisedConversations = true

@ShGKme ShGKme merged commit 79b22c6 into master Jun 22, 2023
20 checks passed
@ShGKme ShGKme deleted the fix/9630/optimize-conversations-update branch June 22, 2023 10:09
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.

🔥 Sever performance/CPU issue with at least firefox due to Talk page rendering
3 participants