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

Consider Optimization to hold onto only essential elements in Parser #227

Open
suyashkumar opened this issue Dec 17, 2021 · 1 comment
Open
Assignees
Labels
enhancement New feature or request

Comments

@suyashkumar
Copy link
Owner

When using the element-by-element API in the Parser, we probably don't need to hold the whole dataset in p.dataset (as I alluded to a bit in #224 and elsewhere). We really only need to hold onto some subset of elements that we need to parse later elements (like PixelData). The rest, we return to the user and the user can decide what they wish to do with them (they may wish to write them out somewhere and then throw them away, for example).

This may help with some memory usage in some usecases for an end user, but because non-PixelData aren't usually super huge, it will be a small to medium size impact.

@suyashkumar suyashkumar added the enhancement New feature or request label Dec 17, 2021
@suyashkumar suyashkumar self-assigned this Dec 17, 2021
@suyashkumar
Copy link
Owner Author

The basic implementation is pretty simple (put something quick up here that I need to cleanup, and add better benchmarks for later https://github.com/suyashkumar/dicom/compare/s/parser-optimize?expand=1), but in the default case where ParseFile or Parse is called (where we need to build and return the entire dataset anyway) we may end up using slightly more memory where we end up storying an additional copy of the metadata elements and the elements that are needed to read future elements. This set should be pretty small though. One option is to put this behavior behind an option in the Parser so that it can only be activated when a user is using Parser in element-by-element mode, but it may not be worth the complexity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant