-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Increase simultaneous requests #2000
Conversation
This all seems very reasonable. Thanks for laying out your case and for taking the time to draft a PR. I'll merge these changes and make sure they're duplicated in our beta branch. |
Thanks @dannyrb! One thing I wanted to change before marking this PR as ready for review was update the package.json in extensions/cornerstone to use the latest version of react-cornerstone-viewport once my PR in that library got merged, as this change currently won't be beneficial unless using the latest react-cornerstone-viewport. If you'd like, I could create another PR to update the package.json to use version 4.0.2? |
👋 @Alex979, that would be great! The PR should:
Thanks again ^_^ |
Thanks for the PR @Alex979 and the nice explanation. This is something that Egor (@pavertomato) also brought up when integrating OHIF with the Google Cloud Healthcare API. I have no problem removing the default and letting people set this themselves at the app level. The tricky thing is that when you have a large number of open requests, depending on the client's connection, the archive and server performance, the latency for each image can increase significantly. Testing needs to take into account the behaviour of the user rather than just total time to load all instances. One thing that I know @pieper and @fedorov want to test for their work with the NCI Imaging Data Commons (which uses a proxy in front of the Google Cloud Healthcare API) is the perceived performance for a user loading a study and then quickly navigating to various locations in a stack, which is typically what we see end users doing. Currently, when the user navigates, we reset the stack prefetch queue to fetch around the image. I'm not sure how the perceived performance is impacted by changes to the maximum simultaneous open requests. |
* feat: 🎸 Update react-vtkjs-viewport usage to use requestPool (OHIF#1984) * feat: 🎸 Update react-vtkjs-viewport usage to use requestPool * Fix import of react-vtkjs-viewport to cornerstone-tools path. * Increase maximum load time of MPR test now we are throttling requests. * Remove debugger Co-authored-by: Erik Ziegler <[email protected]> * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] * fix: Avoid lerna:restore unless we are on Netlify (closes OHIF#1926, OHIF#1996) * wip * fix: Fix incorrect command name in Percy test (OHIF#1999) * perf(stackPrefetch): Added stackPrefetch config with 20 max concurrent requests (OHIF#2000) Co-authored-by: Danny Brown <[email protected]> * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] * feat: 🎸 Filter/promote multiple series instances (OHIF#1533) improve filter/promote to be applied on multiple series instances ✅ Closes: 1532 Co-authored-by: Rodolfo Ladeira <[email protected]> * chore(release): publish [skip ci] - @ohif/[email protected] * fix: Updated react-cornerstone-viewport to version 4.0.2 (OHIF#2001) Co-authored-by: Danny Brown <[email protected]> * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] * fix: 🐛 Fail gracefully on an MPR load error (OHIF#1992) * feat: 🎸 Update react-vtkjs-viewport usage to use requestPool * Fix import of react-vtkjs-viewport to cornerstone-tools path. * Increase maximum load time of MPR test now we are throttling requests. * fix: 🐛 Fail gracefully on an MPR load error * Respond to reviewer comments. * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] * [IDC-1994] Sort series list by SeriesNumber, and sort by same SeriesNumber by date/time. (OHIF#2010) * Sort based on SeriesNumber and SeriesDate/SeriesTime. * Harden, and perform final sort in algorithm if last N entries have the same SeriesNumber. * Switch to insertion rather than sorting as sorting is too slow. Reimplement low priority sorting into new insertion method. * Fix local file viewing. * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] Co-authored-by: James Petts <[email protected]> Co-authored-by: Erik Ziegler <[email protected]> Co-authored-by: ohif-bot <[email protected]> Co-authored-by: Alex Broaddus <[email protected]> Co-authored-by: Danny Brown <[email protected]> Co-authored-by: ladeirarodolfo <[email protected]> Co-authored-by: Rodolfo Ladeira <[email protected]>
* feat: 🎸 Update react-vtkjs-viewport usage to use requestPool (OHIF#1984) * feat: 🎸 Update react-vtkjs-viewport usage to use requestPool * Fix import of react-vtkjs-viewport to cornerstone-tools path. * Increase maximum load time of MPR test now we are throttling requests. * Remove debugger Co-authored-by: Erik Ziegler <[email protected]> * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] * fix: Avoid lerna:restore unless we are on Netlify (closes OHIF#1926, OHIF#1996) * wip * fix: Fix incorrect command name in Percy test (OHIF#1999) * perf(stackPrefetch): Added stackPrefetch config with 20 max concurrent requests (OHIF#2000) Co-authored-by: Danny Brown <[email protected]> * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] * feat: 🎸 Filter/promote multiple series instances (OHIF#1533) improve filter/promote to be applied on multiple series instances ✅ Closes: 1532 Co-authored-by: Rodolfo Ladeira <[email protected]> * chore(release): publish [skip ci] - @ohif/[email protected] * fix: Updated react-cornerstone-viewport to version 4.0.2 (OHIF#2001) Co-authored-by: Danny Brown <[email protected]> * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] * fix: 🐛 Fail gracefully on an MPR load error (OHIF#1992) * feat: 🎸 Update react-vtkjs-viewport usage to use requestPool * Fix import of react-vtkjs-viewport to cornerstone-tools path. * Increase maximum load time of MPR test now we are throttling requests. * fix: 🐛 Fail gracefully on an MPR load error * Respond to reviewer comments. * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] * [IDC-1994] Sort series list by SeriesNumber, and sort by same SeriesNumber by date/time. (OHIF#2010) * Sort based on SeriesNumber and SeriesDate/SeriesTime. * Harden, and perform final sort in algorithm if last N entries have the same SeriesNumber. * Switch to insertion rather than sorting as sorting is too slow. Reimplement low priority sorting into new insertion method. * Fix local file viewing. * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] * [IDC-2017] Harden segmentation import to support more SEGs (OHIF#2024) * fix: 🐛 Upgrade dcmjs version to support more SEGs * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] * [IDC-1939] Debug Dialog part 1 (OHIF#2011) * WIP debug dialog * Rename the p10 downloader extension to debugger extension, add button to toolbar. Deactivate it by default. * Fix unit tests * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] * IDC-1532 multiple series search google (OHIF#2026) * Multiple series search for google cloud adapter. * Revert IDC config. * fix: 🐛 Series filtering on multiple series for google * Revert changes to default config. * Address reviewers comments. * Fixed spelling mistakes * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] * Split query parameter calls. (OHIF#2035) * chore(release): publish [skip ci] - @ohif/[email protected] * [IDC-1672] Highlight SEG segment/ RT structure when click to jump. (OHIF#2034) * Highlight for RTSTRUCT. * SEG temp crosshairs. * Disable RT highlighting for now. * Remove TODO * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] * [IDC-1129] - DICOM Tag Viewer extension (OHIF#2022) * WIP render top level tags. * Add drop down to select series. * Fix errors with type 2 sequences. * WIP swap instance. * Fix formatting and make fullscreen. * Remove debuggers. * Finish formatting. * Fix error with double SeriesNumber deconstruction. * Address reviewer comments. * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] * [IDC-2006] - optional mailTo for debugging extension. (OHIF#2027) * WIP debug dialog * Rename the p10 downloader extension to debugger extension, add button to toolbar. Deactivate it by default. * Fix unit tests * WIP * WIP * Finish mailTo * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] * [IDC-1905] Hover tooltips for Segmentation + RTSTRUCT extension (OHIF#2044) * WIP debug dialog * Rename the p10 downloader extension to debugger extension, add button to toolbar. Deactivate it by default. * Fix unit tests * WIP * WIP * Finish mailTo * tooltop Co-authored-by: Erik Ziegler <[email protected]> Co-authored-by: ohif-bot <[email protected]> Co-authored-by: Alex Broaddus <[email protected]> Co-authored-by: Danny Brown <[email protected]> Co-authored-by: ladeirarodolfo <[email protected]> Co-authored-by: Rodolfo Ladeira <[email protected]>
* feat: 🎸 Update react-vtkjs-viewport usage to use requestPool (OHIF#1984) * feat: 🎸 Update react-vtkjs-viewport usage to use requestPool * Fix import of react-vtkjs-viewport to cornerstone-tools path. * Increase maximum load time of MPR test now we are throttling requests. * Remove debugger Co-authored-by: Erik Ziegler <[email protected]> * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] * fix: Avoid lerna:restore unless we are on Netlify (closes OHIF#1926, OHIF#1996) * wip * fix: Fix incorrect command name in Percy test (OHIF#1999) * perf(stackPrefetch): Added stackPrefetch config with 20 max concurrent requests (OHIF#2000) Co-authored-by: Danny Brown <[email protected]> * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] * feat: 🎸 Filter/promote multiple series instances (OHIF#1533) improve filter/promote to be applied on multiple series instances ✅ Closes: 1532 Co-authored-by: Rodolfo Ladeira <[email protected]> * chore(release): publish [skip ci] - @ohif/[email protected] * fix: Updated react-cornerstone-viewport to version 4.0.2 (OHIF#2001) Co-authored-by: Danny Brown <[email protected]> * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] * fix: 🐛 Fail gracefully on an MPR load error (OHIF#1992) * feat: 🎸 Update react-vtkjs-viewport usage to use requestPool * Fix import of react-vtkjs-viewport to cornerstone-tools path. * Increase maximum load time of MPR test now we are throttling requests. * fix: 🐛 Fail gracefully on an MPR load error * Respond to reviewer comments. * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] * [IDC-1994] Sort series list by SeriesNumber, and sort by same SeriesNumber by date/time. (OHIF#2010) * Sort based on SeriesNumber and SeriesDate/SeriesTime. * Harden, and perform final sort in algorithm if last N entries have the same SeriesNumber. * Switch to insertion rather than sorting as sorting is too slow. Reimplement low priority sorting into new insertion method. * Fix local file viewing. * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] * [IDC-2017] Harden segmentation import to support more SEGs (OHIF#2024) * fix: 🐛 Upgrade dcmjs version to support more SEGs * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] * [IDC-1939] Debug Dialog part 1 (OHIF#2011) * WIP debug dialog * Rename the p10 downloader extension to debugger extension, add button to toolbar. Deactivate it by default. * Fix unit tests * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] * IDC-1532 multiple series search google (OHIF#2026) * Multiple series search for google cloud adapter. * Revert IDC config. * fix: 🐛 Series filtering on multiple series for google * Revert changes to default config. * Address reviewers comments. * Fixed spelling mistakes * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] * Split query parameter calls. (OHIF#2035) * chore(release): publish [skip ci] - @ohif/[email protected] * [IDC-1672] Highlight SEG segment/ RT structure when click to jump. (OHIF#2034) * Highlight for RTSTRUCT. * SEG temp crosshairs. * Disable RT highlighting for now. * Remove TODO * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] * [IDC-1129] - DICOM Tag Viewer extension (OHIF#2022) * WIP render top level tags. * Add drop down to select series. * Fix errors with type 2 sequences. * WIP swap instance. * Fix formatting and make fullscreen. * Remove debuggers. * Finish formatting. * Fix error with double SeriesNumber deconstruction. * Address reviewer comments. * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] * [IDC-2006] - optional mailTo for debugging extension. (OHIF#2027) * WIP debug dialog * Rename the p10 downloader extension to debugger extension, add button to toolbar. Deactivate it by default. * Fix unit tests * WIP * WIP * Finish mailTo * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] * [IDC-1905] Hover tooltips for Segmentation + RTSTRUCT extension (OHIF#2044) * WIP debug dialog * Rename the p10 downloader extension to debugger extension, add button to toolbar. Deactivate it by default. * Fix unit tests * WIP * WIP * Finish mailTo * tooltop * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] * [IDC-2049] Sort Tags in Tag browser split items in sequences, add indent after space. (OHIF#2053) * Sort tag browser, add items, add indent. * Remove debugger. * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] * Update react-cornerstone-viewport for performance improvements (OHIF#2062) * Update react-cornerstone-viewport * Bump react-cornerstone-viewport version. * Comment out tests which always cause problems. * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] * [IDC-2052] PWA start url (OHIF#2070) Change start_url in manifest.json from "./index.html" to "./" Co-authored-by: Woonchan Cho <[email protected]> * chore(release): publish [skip ci] - @ohif/[email protected] * fix: Enable port changes for Docker image using the PORT variable (OHIF#2016) * fix: use SeriesMetadata method instead of property in findMostRecentStructuredReport (OHIF#2038) fixes OHIF#1714 Call getInstanceCount() instead of manually checking length (which does not work) Co-authored-by: Stefan Silviu <[email protected]> * chore(release): publish [skip ci] - @ohif/[email protected] - @ohif/[email protected] - @ohif/[email protected] Co-authored-by: Erik Ziegler <[email protected]> Co-authored-by: ohif-bot <[email protected]> Co-authored-by: Alex Broaddus <[email protected]> Co-authored-by: Danny Brown <[email protected]> Co-authored-by: ladeirarodolfo <[email protected]> Co-authored-by: Rodolfo Ladeira <[email protected]> Co-authored-by: Woonchan Cho <[email protected]> Co-authored-by: Woonchan Cho <[email protected]> Co-authored-by: Nicolas Byl <[email protected]> Co-authored-by: Ștefan Silviu-Alexandru <[email protected]> Co-authored-by: Stefan Silviu <[email protected]>
PR Checklist
@mention
a maintainer to request a reviewHello, I am an intern at Google investigating ways in which to get optimal performance out of medical image viewers that integrate with the Google Healthcare API. One major optimization I've found is increasing the number of simultaneous requests made when fetching a series of DICOM instances/frames. So this PR adds a configuration to
cornerstoneTools.stackPrefetch
to increase the number of simultaneous http requests.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). Increasing the number of simultaneous http requests higher than 6 yields major performance improvements as images load much faster.
Currently, a library used in Ohif react-cornerstone-viewport sets the cornerstoneTools maxSimultaneousRequests to 6, overriding any value that we set ourselves. However, I have opened a pull request to remove this configuration which would allow viewers to set these values to whatever they like. Thus, if that pull request gets merged, then we can set the maxSimultaneousRequests to whatever value we like, and I'll add a commit to update to the latest version of react-cornerstone-viewport.
I did some performance tests comparing the load times of different values of concurrent requests w/ Ohif, when loading a study with 619 instances in it, results below:
It appears that after 20 max simultaneous requests, the added benefits become negligible. So in this PR I have set the maxSimultaneousRequests value to 20. As can be seen there is a very large decrease in load times for the 619 DICOM instances simply by increasing the number of simultaneous http requests, allowing users to view the instances much quicker and with less noticeable lag.
Relevant issues
#820
If anyone would like to test this change before the react-cornerstone-viewport PR gets merged, you can use my forked version of the library which won't override the maxSimultaneousRequests.