-
-
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
perf(segmentation): make segmentation read 3x faster #3577
Conversation
✅ Deploy Preview for ohif-dev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for ohif-platform-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov Report
@@ Coverage Diff @@
## master #3577 +/- ##
==========================================
- Coverage 42.87% 42.81% -0.06%
==========================================
Files 80 80
Lines 1446 1448 +2
Branches 339 340 +1
==========================================
Hits 620 620
- Misses 662 663 +1
- Partials 164 165 +1
Continue to review full report in Codecov by Sentry.
|
Passing run #3421 ↗︎
Details:
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. |
@sedghi can you maybe give a sentence summary of what was done to make performance 3x faster? To be clear, was it the goal to make it 3x faster, or it just became 3x faster as a result of optimizations of the implementation? Are there any known implementation issues that can be resolved with some effort to make it even more faster? |
@fedorov updated |
|
||
let firstPosition = perFrame[0].PlanePositionSequence.ImagePositionPatient.map( | ||
Number | ||
const results = await adaptersSEG.Cornerstone3D.Segmentation.generateToolState( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is much cleaner and easier to understand.
extensions/cornerstone-dicom-seg/src/getSopClassHandlerModule.js
Outdated
Show resolved
Hide resolved
extensions/cornerstone-dicom-seg/src/getSopClassHandlerModule.js
Outdated
Show resolved
Hide resolved
extensions/cornerstone-dicom-seg/src/viewports/OHIFCornerstoneSEGViewport.tsx
Show resolved
Hide resolved
platform/ui/src/components/LoadingIndicatorTotalPercent/LoadingIndicatorTotalPercent.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of smallish changes. Definitely easier to read already.
Looks good. Looks like there are conflicts to resolve prior to merge. I confirmed that #3420 is fixed and that segmentation read is now faster. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved
fixes #3420 and #3523
Context
Move the SEG read into cornerstone3D adapters which was recently merged which has metadata call optimizations for performance
The goal was working with the old implementation of SEG read so that #3420 is fixed. As a result of switching back I also profile the read and saw repetitive metadata call in the PerFrameSequence for loop so precaching two repetitive calls. One here
and another here made the read faster
Changes & Results
remove lots of SEG read code
Testing
everything should behave the same but faster than before
Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals.
Tested Environment