Uppy: Thumbnail generation hangs the browser

[richardwillars]

When I'm uploading 70+ images (nothing particularly big) then the browser stutters and hangs for around 5-10 seconds. I narrowed the issue down to the thumbnail generation code (specifically the bit where it resizes it using canvas.

I managed to improve the problem quite significantly by generating the thumbnails sequentially rather than all at the same time. This is the code I used: https://gist.github.com/richardwillars/7eb86220330068dd184670c690bb4c4b

However, even running it sequentially still makes it hang for a couple of seconds - it's when it gets to certain images it just blocks the UI thread completely. We should probably add something onto the backlog to look at this in the future - I've seen comments about using webworkers etc to do the processing on a different thread.

In the meantime, would you guys be open to a PR that allows the thumbnail generation to be turned off (using a config option)? Our short-term plan at Photobox is to turn it off to improve performance, and then rely on the browser to render the full-size images at thumbnail sizes (this works surprisingly well - I guess it's because the browser rendering runs on a separate thread to JS).

[arturi]

@goto-bus-stop hi, this seems like your turf, what do you think?

[arturi]

I think this options definitely won’t hurt, but maybe we should also do something like if files > X, don’t generate thumbnails, or queue thumbnail generation, or indeed webworkers as we discussed?

[goto-bus-stop]

in my experience the major slow down around thumbnail generation previously was rerenders, which was helped a lot by nanoraf. I was testing with fewer images though (more like 20ish). We're still doing a lot of work just resizing the images.

I think queueing is a good idea. I experimented with something like this in the past:

generatePreview (file) {
  const previous = this.previousThumbnail || Promise.resolve()
  if (Utils.isPreviewSupported(file.type) && !file.isRemote) {
    this.previousThumbnail = previous
      .then(() => Utils.createThumbnail(file.id, thumbnail))
      .then((thumbnail) => this.setPreviewURL(file.id, thumbnail))
      .catch((err) => console.warn(err.stack || err.message))
  }
}

We could also look into using idle callbacks, they're brand new though.

Web workers may still be ideal but the downside to them is that we would have to do the resize manually, since the canvas context cannot be transfered to a worker. it might add a lot to our bundle size to inline a resize algorithm (or use pica).
(aside: apparently pica uses a webassembly implementation now which is super cool!)

[richardwillars]

Spent a few days looking into this and it's pretty tricky as the browsers all still rely on canvas (which isn't supported on webworkers). Once they support off-screen canvas (feature flagged on chrome and FF) then we should be able to do all of the grunt work on a service worker.

I've implemented a queuing system which does help quite a lot. I also moved all the thumbnail code into a plugin and out of core.js and utils.js (for me it made more sense to be there as some users won't want thumbnail generation at all, and it also lays the groundwork for the service worker integration).

https://gist.github.com/richardwillars/e2843119cff74245eab2d162bd1e6483

[goto-bus-stop]

yeah i like having it as a separate plugin--we could still add it by default in the Dashboard (similar to the StatusBar), this looks great!

Once they support off-screen canvas (feature flagged on chrome and FF) then we should be able to do all of the grunt work on a service worker.

that's really interesting, we should keep an eye on that for sure!

[arturi]

thank you, @richardwillars!

yeah i like having it as a separate plugin--we could still add it by default in the Dashboard (similar to the StatusBar), this looks great!

+1 on this

[arturi]

@richardwillars would you submit a PR with your plugin please?

[richardwillars]

It's on my list.. will hopefully get round to it tomorrow 🤞

[richardwillars]

Raised PR at https://github.com/transloadit/uppy/pull/461

[goto-bus-stop]

Closing per #461. thanks!