Skip to content

Use a thread to run ROS executor#1685

Draft
ahcorde wants to merge 1 commit intorollingfrom
ahcorde/rolling/visualizer_manager_thread_ros
Draft

Use a thread to run ROS executor#1685
ahcorde wants to merge 1 commit intorollingfrom
ahcorde/rolling/visualizer_manager_thread_ros

Conversation

@ahcorde
Copy link
Copy Markdown
Contributor

@ahcorde ahcorde commented Mar 19, 2026

No description provided.

Signed-off-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>
@ahcorde ahcorde requested review from mjcarroll and sloretz March 19, 2026 11:16
@ahcorde ahcorde self-assigned this Mar 19, 2026
@ahcorde ahcorde requested a review from asymingt March 19, 2026 11:16
@ahcorde
Copy link
Copy Markdown
Contributor Author

ahcorde commented Mar 20, 2026

@mjcarroll some ideas why this might be usefull

Lower callback latency: callbacks are dispatched immediately instead of waiting up to one 30 Hz tick (~33 ms). This is critical for time-sensitive data such as TF transforms and sensor messages. ║

Update loop never blocked: the Qt render/update loop runs at its full rate without being delayed by executor work. This keeps the UI responsive under load.

Higher throughput: all N messages are processed faster because the executor is not rate-limited to 30 Hz ticks.

@ahcorde
Copy link
Copy Markdown
Contributor Author

ahcorde commented Mar 20, 2026

I made in this branch a benchmark test comparing both solutions.

This is the result

Old spin_some 30 Hz Old spin_some 60 Hz New thread (any rate)
Mean callback latency ~1 690 000 µ ~847 000 µs ~700–5 000 µs
Tick budget stolen 30% (10 ms / 33 ms) 60% (10 ms / 17 ms) 0%
Update loop blocked yes worse never

@ahcorde ahcorde marked this pull request as draft March 20, 2026 14:48
Copy link
Copy Markdown
Member

@asymingt asymingt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this change. Are you certain that it won't introduce any data races on variables accessed from within executor callbacks, now that they are accessed from a separate thread?

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