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

Migrate 2D image cache away from std::vector<float> #967

Closed
veggiesaurus opened this issue Nov 18, 2021 · 2 comments
Closed

Migrate 2D image cache away from std::vector<float> #967

veggiesaurus opened this issue Nov 18, 2021 · 2 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@veggiesaurus
Copy link
Collaborator

We currently use a std::vector<float> for Frame::_image_cache. However, the ability to resize the image cache is not of any use, because a new cache is made for each separate image, and all image channels have the same 2D slice size. The main problem with using a std::vector is that we end up allocating memory and initialising to zero before we fill it with values from disk. This means it will always be slower than something like a simple c-style array.

This is not a problem for small images, but for large images it can become a substantial portion of the initial image load time.

@veggiesaurus veggiesaurus added the enhancement New feature or request label Nov 18, 2021
@veggiesaurus veggiesaurus assigned jolopezl and unassigned pford Dec 3, 2021
@kswang1029 kswang1029 added this to the v3.0b-3 milestone Jan 9, 2022
@jolopezl
Copy link
Contributor

jolopezl commented Jan 21, 2022

@veggiesaurus, do we have any design constraint to resist introducing c-style arrays here in favor of a significant performance gain for large images?

@veggiesaurus
Copy link
Collaborator Author

No, thats totally fine, as long as they are carefully cleaned up. We don't actually ever need to resize the image cache if I recall correctly

confluence pushed a commit that referenced this issue Mar 9, 2022
* replace vector with pointers

* fix style

* remove additional debug printing

* fix compilation

* add range check to BasicStatsCalculator

* simplify BasicStatsCalculator
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

4 participants