Thumbnail lazy-loader is too slow || Use browser-level lazy-loading #6332

New issue

Closed

opened 2021-06-25 14:29:25 +02:00 by infinite-persistence · 4 comments

infinite-persistence commented

2021-06-25 14:29:25 +02:00

(Migrated from github.com)

Issues

Our current implementation of image lazy-loading only fetches the image when it is 20% in the viewport.
- On Mobile, we can only see 2 tiles at a time. So, it's pretty much "scroll, wait 1s, scroll, wait 1s, ..." -- quiet frustrating to use.
- On Desktop, it's just as frustrating to see the homepage constantly filled with placeholders.
Having 120 IntersectionObservers in the frontpage probably isn't very efficient. https://www.bennadel.com/blog/3954-intersectionobserver-api-performance-many-vs-shared-in-angular-11-0-5.htm.
- No direct proof so far, but I'm seeing lots of GC in the performance tab.

Suggestion

Let's try to use the browser-level support instead: https://web.dev/browser-level-image-lazy-loading/ (or was it considered before?)
- It includes the desired "load when near viewport" behavior.
- Less code
- Don't fight the browser that might already be doing it (e.g. in Chrome Android)
- The article was written in 2019 saying "coming soon on Safari". If still not supported, there's a suggested way to fallback to other methods.

## Issues - Our current implementation of image lazy-loading only fetches the image when it is 20% in the viewport. - On Mobile, we can only see 2 tiles at a time. So, it's pretty much "scroll, wait 1s, scroll, wait 1s, ..." -- quiet frustrating to use. - On Desktop, it's just as frustrating to see the homepage constantly filled with placeholders. - Having 120 `IntersectionObservers` in the frontpage probably isn't very efficient. https://www.bennadel.com/blog/3954-intersectionobserver-api-performance-many-vs-shared-in-angular-11-0-5.htm. - No direct proof so far, but I'm seeing lots of GC in the performance tab. ## Suggestion - Let's try to use the browser-level support instead: https://web.dev/browser-level-image-lazy-loading/ (or was it considered before?) - It includes the desired "load when near viewport" behavior. - Less code - Don't fight the browser that might already be doing it (e.g. in Chrome Android) - The article was written in 2019 saying "coming soon on Safari". If still not supported, there's a suggested way to fallback to other methods.

tzarebczan commented

2021-06-25 18:23:53 +02:00

(Migrated from github.com)

This would also fix https://github.com/lbryio/lbry-desktop/issues/6002 hopefully..have been seeing more complaints about it.

DispatchCommit commented

2021-07-01 04:35:36 +02:00

(Migrated from github.com)

Safari is like modern day IE I swear.

https://caniuse.com/loading-lazy-attr

75% global coverage, not supported in safari still.

Safari is like modern day IE I swear. https://caniuse.com/loading-lazy-attr 75% global coverage, not supported in safari *still*.

infinite-persistence commented

2021-07-01 04:46:04 +02:00

(Migrated from github.com)

:( Yeah, several issues:

Need fallback for Safari anyway
Doesn't work on background-image yet -- need to fallback to IntersectionObserver anyways
Chrome's current implementation seems too eager (loads when within ~3000px of viewport). Firefox's is pretty good, though.

Will either drop entirely or mix -- channel thumbnails to use lazy, while tile previews continue to use IntersectionObserver for now.

:( Yeah, several issues: - Need fallback for Safari anyway - Doesn't work on `background-image` yet -- need to fallback to `IntersectionObserver` anyways - Chrome's current implementation seems too eager (loads when within ~3000px of viewport). Firefox's is pretty good, though. Will either drop entirely or mix -- channel thumbnails to use `lazy`, while tile previews continue to use `IntersectionObserver` for now.

DispatchCommit commented

2021-07-01 05:23:14 +02:00

(Migrated from github.com)

Having 120 IntersectionObservers in the frontpage probably isn't very efficient

I don't think this is the case. While there may be a faster way with reducing the amount of listeners, I do not think 120 is a very large amount, and the observer is a non-blocking async function, designed specifically for this type of use case.

If anything, the events and code that fires when an observer becomes visible may be slow, but I don't think the observer is to blame for that.

Seems like potentially premature optimization on this one. The image loading time is still far outweighing any computation time, and once observed, the intersection observer could be removed, and then it doesn't even factor into performance anymore.

> Having 120 IntersectionObservers in the frontpage probably isn't very efficient I don't think this is the case. While there may be a *faster* way with reducing the amount of listeners, I do not think 120 is a very large amount, and the observer is a non-blocking async function, designed specifically for this type of use case. If anything, the events and code that fires when an observer becomes visible *may* be slow, but I don't think the observer is to blame for that. Seems like potentially premature optimization on this one. The image loading time is still far outweighing any computation time, and once observed, the intersection observer could be removed, and then it doesn't even factor into performance anymore.