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

call of writeToBuffer on huge png causing tab freeze #71

Closed
serafimsanvol opened this issue Jul 23, 2024 · 10 comments
Closed

call of writeToBuffer on huge png causing tab freeze #71

serafimsanvol opened this issue Jul 23, 2024 · 10 comments
Labels
bug Something isn't working
Milestone

Comments

@serafimsanvol
Copy link

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 kleisauke added the question Further information is requested label Jul 24, 2024
@kleisauke
Copy link
Owner

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:
#15 (comment)

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:

wasm-vips/lib/vips.d.ts

Lines 8739 to 8742 in 6b83b72

/**
* Quantisation cpu effort.
*/
effort?: number

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

@serafimsanvol
Copy link
Author

serafimsanvol commented Jul 24, 2024

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
Copy link
Owner

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
Copy link
Author

@kleisauke, got it, thanks for the explanation

@serafimsanvol
Copy link
Author

serafimsanvol commented Aug 6, 2024

@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
Copy link
Owner

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
Copy link
Author

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 kleisauke added bug Something isn't working and removed question Further information is requested labels Aug 7, 2024
@kleisauke kleisauke added this to the v0.0.10 milestone Aug 7, 2024
@kleisauke
Copy link
Owner

kleisauke commented Aug 7, 2024

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 5c7d274 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).

wasm-vips/lib/vips.d.ts

Lines 68 to 74 in 5c7d274

/**
* Gets or, when a parameter is provided, sets the number of worker threads libvips' should create to
* process each image.
* @param concurrency The number of worker threads.
* @return The number of worker threads libvips uses for image evaluation.
*/
function concurrency(concurrency?: number): void | number;

@serafimsanvol
Copy link
Author

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

@kleisauke
Copy link
Owner

v0.0.10 is now available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants