Skip to content

Fix https://github.com/jsk-ros-pkg/jsk_recognition/issues/2860 #2866

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mqcmd196
Copy link
Member

close #2860

libsiftfast has so many static global variables, so it is not thread-safe when called from multiple nodelets. This PR adds a global mutex to restrict the access to those variables to make it thread-safe.

Since the entire mutex is taken, if n imagesift nodelet are loaded, the time required to compute each feature is n times longer. But I think it is fine because the node is implemented with ConnectionBasedNodelet. If the user doesn't subscribe, the slow calculation doesn't occur. Making the user code work is much important.

The clean method is to make libsiftfast thread-safe, but I think we should write it from scratch or use OpenCV if we do so.

You can try the bug occurs just reverting 365b2d0 and execute test_two_imagesift.test

@mqcmd196 mqcmd196 requested review from iory and k-okada April 15, 2025 13:46
@mqcmd196 mqcmd196 force-pushed the fix-imagesift-segfault branch from fbeae6a to 08ed039 Compare April 23, 2025 02:23
@mqcmd196
Copy link
Member Author

requests.exceptions.ConnectionError: HTTPConnectionPool(host='jenkins.jsk.imi.i.u-tokyo.ac.jp', port=8080): Max retries exceeded with url: /crumbIssuer/api/json (Caused by NewConnectionError('<urllib3.connection.HTTPConnection object at 0x7f22e150c5d0>: Failed to establish a new connection: [Errno -5] No address associated with hostname',))

@mqcmd196
Copy link
Member Author

CI green

@k-okada
Copy link
Member

k-okada commented Apr 28, 2025

can you compare calculation speed in single thread with and whitout this PR? because libsiftfast is originally developed to pursue high speed processing by several optimization technique, so I'd like to know how this changes the performance.

@mqcmd196
Copy link
Member Author

calculation speed in single thread

What do you mean? Making the nodelet's num_worker_thread=1 or making the libsiftfast's OpenMP flag OFF?

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.

libsiftfast Segmentation Fault when 2 libsiftfast nodelets are loaded then publishe the topics
3 participants