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

urlForSize triggered too many times for api calls during fast scrolling #36

Open
Sherlock1979 opened this issue Feb 16, 2022 · 18 comments

Comments

@Sherlock1979
Copy link

Hi,

First of all thanks for creating this fantastic library. Exactly what I need for my home photo gallery project.

Here is my use case: I load a couple hundred to a couple thousand photo thumbnails to pig.js and I would like to have a great scrolling/viewing experience. However, the images are hosted on a cloud storage provider and I can access them via api requests. Hence, I implemented the api calls to get the links to the images in the urlForSize. Works perfectly.

The problem is that when I scroll quickly to a different part of the image grid, the cloud api server is bombarded with hundreds of requests and it takes a significant amount of time to get the url of the actual images that are on the screen. The wait time can easily be 20-40 sec or more.

Do you have a solution for the above?

With some googling some ideas I could gather:

  1. Don't call heavy js code on scroll events as there is too many of them. Instead implement a timer and batch/filter the api calls on the timer event and not the scroll event.
  2. Find a way to only call the external api once scrolling is settled. I don't know how to do it.
  3. Drop the pending api requests that we no longer need (as we scrolled over them).

You may have a better idea though.

Thanks,
David

@schlosser
Copy link
Owner

For your cloud provider, you're saying that there's no way to generate the final image URL without hitting an API? As in, you're not storing each image size at a predictable URL like mycloudstorageprovider.com/path/to/image.jpg?size=2000?

You could consider pre-computing the entire map and downloading it, e.g. a single JSON response that looks like:

{
  "myImageKey": {
    "20": "https://...",
    "200": "https://...",
    "800": "https://...",
    //..
  },
  //...
}

this would allow you to get the urlForSize quickly.

If you really need to make / cancel these requests onScroll, I'd recommend looking into AbortController, which allows you to cancel fetch() requests.

Part of the reason this library works well on https://feeding.schlosser.io/ is that browsers are "good" at cancelling <img/> requests after the image element is removed from the DOM.

@Sherlock1979
Copy link
Author

Hi,

For your cloud provider, you're saying that there's no way to generate the final image URL without hitting an API? As in, you're not storing each image size at a predictable URL like mycloudstorageprovider.com/path/to/image.jpg?size=2000?

Yes, that is correct. The path is generated on the fly (with an expiry timestamp and in a way that it is different for each requestor based on their ip address).

Thanks for your idea on pre-calculating all urls. I just gave it a go and there are two problems with it:

  1. I can supply all image IDs to the api but if I request many images at a time then I receive a net::ERR_EMPTY_RESPONSE. I guess the reason might be that my GET request is too long.
  2. I can probably get around this limitation by slicing the image list to smaller chunks and send in more api requests. I tried with a slice of about 300 images and the response time from the server was about 9 seconds. So if I'd like to display let's say 3000 images then the initial load time might be 90 seconds which is not really acceptable.

I will look at AbortController now and will come back to you soon.

Cheers,
David

@Sherlock1979
Copy link
Author

With AbortController I have two questions

  1. Would it work with XHR requests or only fetch?
  2. How should I know which requests to abort and which ones to keep? It seems a bit messy.

In relation to the above, the browser might have already dropped the requests but the async api call is still there.

I think the better approach would be to prevent these api requests from firing. For example, would there be a way to just call the api for the images that we need once we stopped scrolling?

@schlosser
Copy link
Owner

Ah, I follow now, yes. AbortController only works with fetch, but yes if the issue is on the server getting overloaded (vs the client being overloaded), then preventing the calls make sense.

There is already a timeout that intended to prevent this here: https://github.com/schlosser/pig.js/blob/master/src/pig.js#L778-L782

You could try increasing that timeout to something like 300-500ms, and see if that allows the images to load faster. Does that work?

@Sherlock1979
Copy link
Author

This sounds promising. I changed line #823 from 100 to 1000:
}.bind(this), 1000);

Unfortunately I am not sure if I see any improvements. I have a console.log() within my urlForSize function and I see that it is getting triggered several times while I am continuously scrolling up and down with the scrollbar.

@schlosser
Copy link
Owner

Are you able to measure / log exactly which images are being loaded unexpectedly to be sure that unnecessary ones are being loaded? It's worth noting that the library does render images outside the viewport, to make scrolling more smooth (see diagram here: https://github.com/schlosser/pig.js/blob/master/src/pig.js#L534-L590).

Can you tell if, say you are scrolling 10,000 pixels, an image at the 5000 pixel mark is being loaded even though you scrolled way past in in certainly less than 1000ms? (Your code change looks right to me.)

@schlosser
Copy link
Owner

if it comes to that, you could implement a scroll speed measure by using the lastYOffset (https://github.com/schlosser/pig.js/blob/master/src/pig.js#L655) and also capturing lastYOffsetCaptureTime and doing a speed calculation with (lastYOffset - currentYOffset) / (lastYOffsetCaptureTime - currentTime).

@Sherlock1979
Copy link
Author

Sherlock1979 commented Feb 16, 2022

Please ignore the below. I think my pig.js was cached so the timer was still 1 sec.
Let me play with it more and I will come back to you on my findings.

Hmmm, maybe I don't get the concept fully. Now I changed the timer to 10000 ms (10 sec). I wait for the initial load of the images and for pig.js to fully settle. Once I do not see any requests in the console.log, I clear the console window and start scrolling up and down without stopping. While I scroll, I can see new urlForSize requests triggering, much sooner than the 10 seconds limit.

What am I doing wrong?

@schlosser
Copy link
Owner

Hmm, could be a bug in pig. What's supposed to happen is _doLayout should be getting called as you scroll, and images outside the viewport should have .hide() called on them, which would remove them from the DOM and set existsOnPage to false. Could you log to verify where hide is getting called, or not? For example, are you seeing the API requests immediately? or only starting 10s after scrolling starts up again?

@schlosser
Copy link
Owner

schlosser commented Feb 16, 2022

My cursory testing seems to work: if I change the 100ms to 5000ms and scroll down the page at a steady, speedy rate for 10-15s, I see no images loading, but then if I pause for 5s, images pop-in.

@Sherlock1979
Copy link
Author

Sherlock1979 commented Feb 16, 2022

Ah, now I understand how the timer should work. So I can test it in a consistent way. I set the timer to 5s.

I started scrolling down at a steady speedy rate for 10 seconds and no requests were triggered. So that's great.

However, if I repeat the experiment but instead of scrolling to one direction I keep on changing the direction (scrolling up and down) then urlForSize starts to be triggered. ... and it seems that as soon as I change direction, urlForSize is triggered instantly (not just after 5s).

@schlosser
Copy link
Owner

schlosser commented Feb 16, 2022 via email

@Sherlock1979
Copy link
Author

Sherlock1979 commented Feb 16, 2022

Ah yes, makes sense, that's because hide() is not being called. Do you
think that scroll pattern is reasonable to test against?

Well, I plan to implement a small indicator to show where you scrolled to. Similar to how Google Photos shows you the date or the month of where you scrolled to. So I expect my user to scroll down and realise that he scrolled down too much and start scrolling up (without looking at the images). It would be great to hold back loading the images until up/down scrolling is completely settled and the user found the right 'spot'.

If an image is in the viewport for more than, say 1000ms, I'd imagine you would want it to load. You could also make the primary and secondary buffers smaller (even 0) so that images are more likely to leave the "should load" boundaries.

I am not sure I fully understand this part of your message. :)

@Sherlock1979
Copy link
Author

... and my other observation is that the 'steady, speedy scrolling' is not possible on mobile browsers. So while I can replicate your test and keep on scrolling down in a steady, speedy pace on my laptop, I cannot do it on my Android mobile with Chrome. There I see that urlForSize is triggered continuously while I scroll. It does not even wait 5 seconds.

@schlosser
Copy link
Owner

I mean instantiating Pig with options:

new Pig(imageData, {
  primaryImageBufferHeight: 0,
  secondaryImageBufferHeight: 0,
});

Give that a try! I think it will be more aggressive about not loading images.

@schlosser
Copy link
Owner

As for this not working on Android, I'm not sure why that would be the case. You're sure you have your updated code with the 5000ms timeout, and new images are loaded as soon as you scroll?

@Sherlock1979
Copy link
Author

I will give it a try soon. Thanks.

Meanwhile I have optimised my api requests in a way that I cache the filenames that I already received and when urlForSize is called for the same file again then I don't need to reach out to the api. So the overall experience is better than before.

With mobile browsers the problem may be that it is very hard to consistently scroll to one direction (e.g. down). As you just swipe with your fingers it is quite easy to scroll to the other direction a bit. And at that point in time pig starts requesting urls instantly (even without waiting 5 secs).

So I think my only outstanding question is: can you make pig not start requesting for urls when the scrolling direction change? The behaviour should be consistent with scrolling to one direction only. What do you think?

Cheers,
David

@schlosser
Copy link
Owner

The configuration I posted should reduce that. The reason that scrolling direction triggers lots of loading is because of the large primaryImageBufferHeight, which attempts to preload images "in front of where the user is scrolling". Disabling that should help. Let me know how that goes!

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

No branches or pull requests

2 participants