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

Optimize distortion calibration by 1.25x for certain problems #1285

Merged
merged 5 commits into from
Nov 10, 2022

Conversation

p12tic
Copy link
Contributor

@p12tic p12tic commented Oct 12, 2022

I've accidentally noticed that distortion calibration takes a lot of time in std::vector operations. Closer inspection revealed that we're copying data unnecessarily when setting distortion parameters. This PR optimizes this operation by giving the caller an option to supply a callback with which appropriate number of parameters will be initialized with all the usual error checking.

On a machine with AMD 2990WX (with turbo boost disabled) this PR improves the run-time of test_calibration_distortion test from ~3.9s to ~3.1s.

Cost evaluation code copies the distortion params set to acquire its
size and copy the data back in. The amount of copies can be reduced by
storing data in-place.
Currently when the caller wants to acquire the number of parameters they
call getParams() which is expensive as it builds the parameter set from
scratch. This can be improved by implementing a function which only
returns the parameter count.
Cost evaluation code copies the distortion params set to acquire its
size and also introduces an unnecessary vector copy to put the data in.
The amount of copies can be reduced by storing data in-place.
Cost evaluation code initializes a new std::vector as a temporary during
cost copying. This can be improved by storing data in-place.
@fabiencastan fabiencastan added this to the 2.5.0 milestone Nov 10, 2022
@fabiencastan fabiencastan merged commit 7fac36a into alicevision:develop Nov 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants