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

added lru caching for faster undistortion #22

Merged
merged 1 commit into from
Sep 7, 2023

Conversation

originlake
Copy link

To improve processing speed for undistortion
mapillary#1007

@pierotofy
Copy link
Member

Thanks @originlake , (sorry it took so long to review this), I think there's some code in this PR that is not related to the proposed LRU-caching improvement?

Could you also post information about the expected speed-up? (How much faster does undistortion happen with the proposed changes?)

@originlake
Copy link
Author

When compiling opensfm c++ module in debug mode, there will be a linking error without the change in opensfm/src/robust/src/similarity_model.cc. I can remove it if not needed. But it is a problem when trying to debug the c++ modules.

Some time ago this branch introduced imageFilter argument but Opensfm's command line tool is not updated accordingly, if you run bin/opensfm undistort dataset_path in command line, it will raise an error due to this. The command line tool for undistortion is not used in ODM so it's not a problem, I fixed it because I'm using the command line tool for debugging.

def run_dataset(
data: DataSet,
reconstruction: Optional[str] = None,
reconstruction_index: int = 0,
tracks: Optional[str] = None,
output: str = "undistorted",
imageFilter: Callable[[str, np.ndarray], np.ndarray] = None,
skip_images: bool = False,
) -> None:

About performance, ideally, the time cost of camera_mapping calculation is around 2 times the following remap. So in a single-threaded processing, you can expect around 60% time cost reduction. But in multithreading processing, it gets complicated. Each thread takes a lot longer compared to single-threaded processing and I can't observe consistent speed improvement there. I guess it is bottlenecked by other stuff like CPU caching, one image could easily fill up the L3 cache, and there are multiple images racing to consume the cache, the CPU is less efficient even if it has multiple cores running. I can't give a good measurement as it's more dependent on RAM speed, cache size, image size, etc. In my test environment, with 5000x4000 images and 8 cores, I can only observe a 10 to 20% time cost reduction.

@pierotofy
Copy link
Member

That's all right, was just trying to understand the scope of the changes. No modifications needed. Thanks for fixing these issues as well as the LRU cache improvements.

I will test this more thoroughly in the upcoming days and merge it afterwards.

@pierotofy
Copy link
Member

Looks great, thanks @originlake ! 🙏

@pierotofy pierotofy merged commit fea7f89 into OpenDroneMap:319 Sep 7, 2023
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