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

Minor Gallery Rendering Speed Up (#2344) #2345

Closed
wants to merge 3 commits into from

Conversation

wladimirleite
Copy link
Member

Closes #2344.

@lfcnassif
Copy link
Member

lfcnassif commented Oct 20, 2024

Thank you @wladimirleite for this optimization!

But about the first commit here, maybe moving the thumb index read to outside the thread pool makes reading thumbs from index sequential instead of parallel (may be better noticed if the index is on spinning disk). I think it also makes large gallery scrolls (holding the mouse pressed on the scroll bar and moving it up/down) slower, because old cell renderings can't be skipped.

And about the flickering mentioned in the issue, not sure (I'm testing from a remote desktop connection), but I think it is caused by master behavior to always render "..." (image loading label) in the cell before rendering the new image itself. I thought that loading label would be good so the user knows the image is going to be rendered and is not a blank image, that may be better noticed on spinning disks, and on faster SSD disks that could cause some flickering effect because it is rendered very fast...

PS: As I said, I'm testing from a remote connection, so my perception could be totally different from a local case test...

@wladimirleite
Copy link
Member Author

Good points! I will take a closer look on this.

@wladimirleite
Copy link
Member Author

I made some tests using a large case in an old USB 2.0 external HDD, and indeed the responsiveness of gallery scrolling becomes clearly worse. It is better to keep the current behavior, as it is more general. The changes that I made did help when the case is on a fast disk and the use case is viewing a lot of thumbnails at a time, and moving with the page down/up.

@lfcnassif
Copy link
Member

lfcnassif commented Oct 20, 2024

Thank you @wladimirleite for testing on a spinning disk! If you want to keep the other changes of this PR, they will be welcome. And out of curiosity, how page up/down behaved on a spinning disk after these changes? I agree it is an use case more important than randomly scrolling in the gallery.

@wladimirleite
Copy link
Member Author

wladimirleite commented Oct 20, 2024

Page down/up usage seems a bit better in terms of flickering, but sometimes there is a clear responsiveness delay.
About the other changes, although they improve the code a bit (avoiding creating some objects, some synchronized blocks etc.), no visible speed up, both using slow or fast storage for the case (the gain is a few orders of magnitudes lower than the time taken by the remaining part of the code), so I prefer to keep things as they were.
Anyway, thanks @lfcnassif for testing the changes and the feedback!

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

Successfully merging this pull request may close these issues.

Improve gallery rendering speed
2 participants