Skip to content

Minor optimization: emplace_back neighbors #3086

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

Conversation

reunanen
Copy link
Contributor

@reunanen reunanen commented Jun 8, 2025

I maintain an application that calls label_connected_blobs a lot. Noticed while profiling that the code that collects a point's neighbors is a bit of a hotspot there, and it seems that this small (and hopefully rather uncontroversial) change makes a measurable (even if not huge) difference.

@davisking davisking merged commit 81cd1f1 into davisking:master Jun 9, 2025
9 checks passed
@davisking
Copy link
Owner

Huh, really? Both of those should end up turning into the same machine code. It's cleaner the new way, so a good change regardless. But are you sure it's really faster? I would only expect a difference if compiled without compiler optimizations and/or with DLIB_ASSERT enabled.

@reunanen reunanen deleted the minor-optimization-emplace_back-neighbors branch June 9, 2025 05:38
@reunanen
Copy link
Contributor Author

reunanen commented Jun 9, 2025

But are you sure it's really faster?

Hmm, good question. I did measure, and I did look at the profiler reports before and after the change – but not too rigorously perhaps.

Also, this (legacy) project uses an older version of the compiler, which may explain something (?). Also, optimizations should be enabled and assertions disabled, but I can't really say I triple-checked everything.

I could go back and do more measurements if needed, but I guess that's not really necessary.

@davisking
Copy link
Owner

davisking commented Jun 9, 2025

Na, no need. Just curious.

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