kleisauke / wasm-vips

libvips for the browser and Node.js, compiled to WebAssembly with Emscripten.
https://kleisauke.github.io/wasm-vips/
MIT License
519 stars 26 forks source link

call of writeToBuffer on huge png causing tab freeze #71

Closed serafimsanvol closed 3 months ago

serafimsanvol commented 4 months ago

Hey, thanks once again for your library. I believe I'm facing a bug. Specifically, library doesn't finish command if I call writeToBuffer for a PNG file with parameters palette and Q.

Example of the same command but via vips CLI (works well)

vips pngsave sample_5184×3456.png worked.png --Q 50 --vips-progress

Example of code that's freezing on the playground

let im = vips.Image.newFromFile('owl.png');
// Finally, write the result to a blob
const t0 = performance.now();
const outBuffer = im.writeToBuffer('.png', {
    Q: 50,
    palette: true
});
const t1 = performance.now();

const blob = new Blob([outBuffer], { type: 'image/jpeg' });
const blobURL = URL.createObjectURL(blob);
const img = document.createElement('img');
img.src = blobURL;
document.getElementById('output').appendChild(img);

Link to the file that's causing the problem, can't attach here because it has 35+mb in size

Also, the UI is totally freezing when writeToBuffer processes any images, but for bigger images, it's more visible, I understand it's basically CPU calculations, but I can't even add a loader because it won't spin as the UI is freezing, if it's possible to do something with this behavior would really appreciate suggestions, thanks

kleisauke commented 4 months ago

If your image processing logic doesn't require access to the DOM, you can offload the work from the UI thread to a web worker and thereby reducing DOM blocking. See for example: https://github.com/kleisauke/wasm-vips/issues/15#issuecomment-1825631671

Note that image quantization is generally a time-consuming operation. You can control the CPU effort spent on improving lossy palette-based PNG output by passing the effort argument: https://github.com/kleisauke/wasm-vips/blob/6b83b72ef69c7c17755d8533d290909d8e98408e/lib/vips.d.ts#L8739-L8742

The default value is 7, with acceptable values ranging from 1 (fastest/largest) to 10 (slowest/smallest).

serafimsanvol commented 4 months ago

Thanks, @kleisauke! I'll check web worker approach. And what do you think about writeToBuffer freezing problem? Can you reproduce it? (bug when it freezes when processing huge png)

kleisauke commented 4 months ago

The UI freeze issue is because some pthread APIs in Emscripten uses a busy-wait on the main browser thread, which can make the browser tab unresponsive. This cannot be addressed; see for more details: https://emscripten.org/docs/porting/pthreads.html#blocking-on-the-main-browser-thread

This issue does not occur when wasm-vips is initialized on a web worker. However, the playground cannot utilize web workers because it requires access to the DOM.

serafimsanvol commented 4 months ago

@kleisauke, got it, thanks for the explanation

serafimsanvol commented 3 months ago

@kleisauke, so I created a worker and the freezing was gone, but I still have the same issue with the huge files, it just never ends up processing. This file And here is my repo where you can reproduce it worker call to worker

Thanks

kleisauke commented 3 months ago

The reproduction works perfectly for me on both Chrome and Firefox. I cloned the repository, ran npm install && npm run dev, and then uploaded and compressed the image without any issues.

Additionally, I performed a sanity check using this gist: https://gist.github.com/kleisauke/823804c2b5598442de4334753e215199

Which browser are you testing this on? And what does this line print in the JavaScript console?

serafimsanvol commented 3 months ago

Hmm. I'm using Chrome 127.0.6533.89 (arm64), mac os 13.0, m1 16 ram It returns 10 added this parameter https://gist.github.com/kleisauke/823804c2b5598442de4334753e215199#file-es6-worker-js-L9 image And it stucks here

kleisauke commented 3 months ago

It appears that postMessage() still queues a task on the current thread's event loop in Chrome, potentially causing a deadlock in case it gets blocked. https://wpt.fyi/results/workers/postMessage_block.https.html?label=master&label=experimental&aligned

This specific Chromium bug is being tracked here: https://issues.chromium.org/issues/40687798

Furthermore, it has been noticed that new Worker() isn't currencly occurring in parallel, preventing libvips from safely spawning additional threads on the web, even within a web worker. https://wpt.fyi/results/workers/Worker-creation-happens-in-parallel.https.html?label=experimental&label=master&aligned

These implementation bugs are being tracked at: https://bugzilla.mozilla.org/show_bug.cgi?id=1888109 https://bugs.webkit.org/show_bug.cgi?id=271756

I just pushed commit 5c7d2746daef2634eb76016e36aa539f638a0fac that will reduce concurrency by default to 1 on the web. This will be in v0.0.10, thanks for reporting this!

In the meantime, you can get workaround this issue by manually lowering concurrency to 1, by calling vips.concurrency(1). https://github.com/kleisauke/wasm-vips/blob/5c7d2746daef2634eb76016e36aa539f638a0fac/lib/vips.d.ts#L68-L74

serafimsanvol commented 3 months ago

@kleisauke, thanks for your help, with concurrency set to 1 now it works

kleisauke commented 3 months ago

v0.0.10 is now available.