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

fix: copy buffer to prevent oom in workers #454

Merged
merged 1 commit into from
Jun 14, 2023

Conversation

Ouwen
Copy link
Contributor

@Ouwen Ouwen commented Jun 28, 2022

Large uncompressed multiframe data will send the full
arraybuffer in a typed view. For large uncompressed data
such as tomo these will result in OOM issues. This fix
pre-slices the typed array view so only used buffer
is sent via worker.postMessage.

Large uncompressed multiframe data will send the full
arraybuffer in a typed view. For large uncompressed data
such as tomo these will result in OOM issues. This fix
pre-slices the typed array view so only used buffer
is sent via worker.postMessage.
@netlify
Copy link

netlify bot commented Jun 28, 2022

Deploy Preview for cornerstone-wado-image-loader ready!

Name Link
🔨 Latest commit f793009
🔍 Latest deploy log https://app.netlify.com/sites/cornerstone-wado-image-loader/deploys/62bb2e76b7e57e000a4f031b
😎 Deploy Preview https://deploy-preview-454--cornerstone-wado-image-loader.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@dereklukacs
Copy link

The preview is not loading for me:

Uncaught (in promise) DOMException: Failed to execute 'importScripts' on 'WorkerGlobalScope': The script at 'blob:https://deploy-preview-454--cornerstone-wado-image-loader.netlify.app/888.bundle.min.worker.js' failed to load.
    at B.f.i (blob:https://deploy-preview-454--cornerstone-wado-image-loader.netlify.app/cc21dc5d-801e-4e29-8d21-aeb5b116536c:1:795575)
    at blob:https://deploy-preview-454--cornerstone-wado-image-loader.netlify.app/cc21dc5d-801e-4e29-8d21-aeb5b116536c:1:794747
    at Array.reduce (<anonymous>)
    at B.e (blob:https://deploy-preview-454--cornerstone-wado-image-loader.netlify.app/cc21dc5d-801e-4e29-8d21-aeb5b116536c:1:794725)
    at blob:https://deploy-preview-454--cornerstone-wado-image-loader.netlify.app/cc21dc5d-801e-4e29-8d21-aeb5b116536c:1:807294
    at new Promise (<anonymous>)
    at new I (blob:https://deploy-preview-454--cornerstone-wado-image-loader.netlify.app/cc21dc5d-801e-4e29-8d21-aeb5b116536c:1:120704)
    at yA (blob:https://deploy-preview-454--cornerstone-wado-image-loader.netlify.app/cc21dc5d-801e-4e29-8d21-aeb5b116536c:1:807268)
    at blob:https://deploy-preview-454--cornerstone-wado-image-loader.netlify.app/cc21dc5d-801e-4e29-8d21-aeb5b116536c:1:807529
    at a (blob:https://deploy-preview-454--cornerstone-wado-image-loader.netlify.app/cc21dc5d-801e-4e29-8d21-aeb5b116536c:1:1528)

@muhammedmokbel
Copy link

How can I use this solution to handle out of memory Problem ?

@swederik
Copy link
Member

swederik commented Dec 14, 2022

Thanks @Ouwen! This solves a big problem and we definitely want it merged.

Some feedback before we can merge this, though:

  • Add a check if SharedArrayBuffer exists and if the window.crossOriginIsolated is true. If those are true, use a SharedArrayBuffer for the pixel data for WADO-URI. Then it doesn’t need to be transferred to the worker at all. We can run the check once on library initialization.
  • If the check is false, use .slice() to copy the buffer. That means this function getUncompressedImageFrame should have a third argument for “copyPixelData” or some other name (default false) which should enable the usage of .slice(). For WADO-RS we do not want this behaviour. We only want it when SharedArrayBuffer is not supported AND it is WADO-URI or a local DICOM file.

Tagging @jmannau because he's asked about this in Slack.

@jpotwarka
Copy link

The fix works great for me in the given example with a 293 slice US

@justvamp
Copy link

justvamp commented Mar 2, 2023

Hello, this is a great PR as it solves our problem.
Any chance of it to be finished and merged?
Thank you!

@OzgeYurtsever
Copy link

Hi all,
It looks like this fix may solve our memory crash problem in Windows machines. Any chance of merging it soon?
Thank you!

@sedghi
Copy link
Member

sedghi commented Jun 14, 2023

🎉 This PR is included in version 4.13.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

dougyau pushed a commit to dougyau/cornerstoneWADOImageLoader that referenced this pull request Sep 29, 2023
Large uncompressed multiframe data will send the full
arraybuffer in a typed view. For large uncompressed data
such as tomo these will result in OOM issues. This fix
pre-slices the typed array view so only used buffer
is sent via worker.postMessage.
laura-borghesi-lum00n added a commit to dvisionlab/cornerstoneWADOImageLoader that referenced this pull request Mar 20, 2024
 -restored webWorker.js and WebWorkerManager.js to the ones at commit cornerstonejs#454
 -modified convolveTask and sleepTask accordingly
 -modified imports in html ../../dist/cornerstoneWADOImageLoader.bundle.min.js
 -modified webpack configuration port and static path
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants