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

added MAX_CONCURRENCY limit to prevent crashes on bigger machines #2161

Merged
merged 2 commits into from
Nov 30, 2020

Conversation

matthiasg
Copy link
Contributor

As each webworker costs RAM and machines with more cores are available (like AMD puts out or XEON) this can and does easily crash the processes.

Added a limit of six as a variable, but it could also be defined in a way overridable during the build process.

PS: In general, many more places in the code would benefit from BUILD time variables.

@stale
Copy link

stale bot commented Nov 30, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Stale 🥖 label Nov 30, 2020
@stale stale bot removed the Stale 🥖 label Nov 30, 2020
@matthiasg
Copy link
Contributor Author

This change is quite important for high core count machines, is there anything we can improve to allow for merging ?

@swederik
Copy link
Member

swederik commented Nov 30, 2020

It looks fine to me, but I have no high-core machines to test it on so I'll have to trust you. I can easily imagine that you could hit memory limits if you started up 20 webworkers for decoding.

Sorry for the delays, we are a small team and at present do not have funding dedicated for support / maintenance. That will be changing shortly though!

@swederik swederik merged commit d2e777b into OHIF:master Nov 30, 2020
@matthiasg
Copy link
Contributor Author

matthiasg commented Nov 30, 2020

@swederik thanks ! Great to hear about more manpower.

We are in the process of integrating OHIF into our own platform and in the process trying to fix any bugs or extend to make it more general. But we obviously don't want to step into anybodys toes as we don't know you or your long term design goals yet.

We do have experience in DICOM viewing, due to our previous PACS and DICOM Viewer product lines, but have extended into a more general direction and think that putting effort into OHIF/Viewer ourselves makes more sense.

PS: We have machines with 24 and more cores, thus the need for this limit :)

Punzo pushed a commit to Punzo/Viewers that referenced this pull request Dec 3, 2020
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

Successfully merging this pull request may close these issues.

2 participants