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 unionfind #294

Merged
merged 1 commit into from
Oct 30, 2023

Conversation

bouk
Copy link
Contributor

@bouk bouk commented Oct 26, 2023

I figured out that a big part of the union find time spent was initializing the data array. I've optimized it by using two memsets instead of an array, which saves ~4% for an image sized 4896×3264

Before

Screenshot 2023-10-26 at 13 42 26

After

Screenshot 2023-10-26 at 13 42 13

@christian-rauch
Copy link
Collaborator

Is this essentially saving you 5ms on the 4896×3264 image? That's a nice improvement.

Could you explain in the commit message a bit what the change is doing and why it improves the performance? Glancing over this, Isee it's moving the parent and size from the struct ufrec into the struct unionfind, turning them into a point for arrays and then initialising all elements in the list at once instead of looping over the array of struct ufrec?

@bouk
Copy link
Contributor Author

bouk commented Oct 27, 2023

Expanded the commit message a bit. The savings is indeed from using memset instead of a for loop

@christian-rauch
Copy link
Collaborator

Can you remove the concrete ~4% from the commit message again? I guess this value depends a lot on the image resolution and is not the same improvement in all cases.

* Split up data into parent and size arrays
* Use memset to initialize parent and size arrays in one go instead of
  iterating
@bouk
Copy link
Contributor Author

bouk commented Oct 30, 2023

Sure, removed

@christian-rauch
Copy link
Collaborator

Thanks!

@christian-rauch christian-rauch merged commit b5e6e56 into AprilRobotics:master Oct 30, 2023
11 checks passed
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