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

Improve performance #5835 #6291

Draft
wants to merge 3 commits into
base: edge
Choose a base branch
from

Conversation

math-GH
Copy link
Contributor

@math-GH math-GH commented Apr 12, 2024

Closes #5835

Changes proposed in this pull request:

  • 1s => 0s waiting

How to test the feature manually:

  1. mark an article as read
  2. see that it is faster now

Pull request checklist:

  • clear commit messages
  • code manually tested

@math-GH math-GH added the UX User experience label Apr 12, 2024
@math-GH math-GH added this to the 1.24.0 milestone Apr 12, 2024
@Frenzie
Copy link
Member

Frenzie commented Apr 12, 2024

A timeout of 0 is meaningless. ;-) That's equivalent to removing the entire asBatch boolean and related functionality. The batching is for performance reasons. See #2199.

@math-GH
Copy link
Contributor Author

math-GH commented Apr 12, 2024

0.1s would be better than 1s or 0s?

@Alkarex
Copy link
Member

Alkarex commented Apr 12, 2024

When the mark_read() is called, check the asBatch parameter. The idea is not to flood the server with requests but pool them (especially in the case of marking as read during scroll).
Manually clicking on the envelope to mark an article as read is not using the batch logic, so the code you have changed is not even called.
How do you mark an article as read? And please check in the Web browser debugger which code path is triggered.

@Alkarex
Copy link
Member

Alkarex commented Apr 12, 2024

Ah right, now I see that it was to address #5835
But the proposal was to make the logic smarter, not to remove it :-)

@Frenzie
Copy link
Member

Frenzie commented Apr 12, 2024

Manually clicking on the envelope to mark an article as read is not using the batch logic, so the code you have changed is not even called.

That sounds a bit suspect. That means that clicking 10 icons in a row isn't batched while pressing j or k 10 times in a row is. (Which is indeed the case.) It would seem that either the former should be too or the latter shouldn't either.

@Alkarex Alkarex modified the milestones: 1.24.0, 1.25.0 Apr 15, 2024
@math-GH math-GH marked this pull request as draft May 16, 2024 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UX User experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve performance of batch mark as read
3 participants