Remove stackPrefetch configuration to let apps configure themselves

[Alex979]

Hello, I am an intern at Google currently investigating ways in which to get optimal performance out of medical image viewers that integrate with the Google Healthcare API. This pull request is aimed to help open source medical image viewers get better performance when using this viewport component, by removing the stackPrefetch configuration so that viewers can set the maximum number of simultaneous requests to their preferred setting.

With the introduction of HTTP/2, there is no longer a limit of 6 simultaneous requests to a specific origin. Instead, one connection is made to a given origin and a hypothetically infinite number of requests can be made through that one connection (although browsers such as chrome do eventually limit these connections which appears to be around 100 requests). In practice, changing the maxSimultaneousRequests to values as high as 50 or 75 yields a significant increase in performance on existing medical image viewers such as Ohif. Additionally, cornerstoneTools already has defaults set for this value that correspond to different browsers, so there should be no need to set this value to 6 manually.

Having this configuration set in the viewport means that it will overwrite the existing configuration of viewers that may want to set the max simultaneous requests to a larger value, which is why I am proposing that these lines be removed so that medical image viewers that utilize this library can set these options themselves outside of the component to find the settings that yield the best performance for them.

I have tested this change with Ohif and the example pages and the viewport still functions as expected. I also tested with different values for preserveExistingPool and maxImagesToPrefetch. However, as I am not 100% familiar with this codebase, if it is essential to the functionality of this viewport that maxImagesToPrefetch is set to Infinity and preserveExistingPool is set to false, then these values can still be set and the maxSimultaneousRequests field can be left blank, as seen in the setConfiguration method in cornerstoneTools. So the below solution could also be used if these two values need to be set to their defaults:

cornerstoneTools.stackPrefetch.setConfiguration({
  maxImagesToPrefetch: Infinity,
  preserveExistingPool: false,
});

[dbousamra]

Following this keenly. I am a GCP healthcare API customer, and currently looking at optimizing my image viewer. I use react-cornerstone-viewport, but tbh I am tempted to switch to raw cornerstone.js and write my own wrapper.

[Alex979]

@JamesAPetts @dannyrb I notice you both have merged many PRs in the past, so I was wondering if either of you could take a look at this or assign someone to it. Thank you!

[dannyrb]

Hello @Alex979 👋

Thanks for taking the time to draft a PR and provide such a thorough explanation.

Cornerstone/OHIF are currently between grants, so most forward progress is through volunteers (hence our delay in responding). I will take a quick look tonight, but this looks like a clear win.

Best,
DB

[dannyrb]

@dbousamra, as this is just a wrapper, if you're knowledgeable of cornerstone and need to deviate/extend the existing features, it may make more sense to explore something more fitting/streamlined for your solution.

If there are general improvements you would like to suggest, or specific problems you're facing that aren't being solved; please don't hesitate to create an issue. Unfortunately, limited funding and my own personal time may delay a response. In some instances, we've granted frequent contributors read/write access. If that's something you're interested in, we can discuss further.

We also have a cornerstone/OHIF/dcmjs Slack that's invite only where it's a bit easier to nab a core contributors attention. If you'd like an invite to it, let me know by sending an email to: [email protected]

[dannyrb]

@Alex979, to your point, the defaults are already declared here:

• https://github.com/cornerstonejs/cornerstoneTools/blob/master/src/util/getMaxSimultaneousRequests.js

All we're really accomplishing by setting the configuration in this library is blowing out changes to the configuration made by an external source. That sounds more like a bug than a feature to me.

[ohif-bot]

🎉 This PR is included in version 4.0.2 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀