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.

Sign up for GitHub

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

Jump to bottom

Add implementation of spin_some for MultiThreadedExecutor #85

Closed
jacquelinekay opened this issue Aug 13, 2015 · 3 comments
Closed

Add implementation of spin_some for MultiThreadedExecutor #85

jacquelinekay opened this issue Aug 13, 2015 · 3 comments
Labels
enhancement New feature or request

Comments

@jacquelinekay
Copy link
Contributor

Currently spin_some has a single-threaded implementation in Executor that gets inherited by the child classes. The implementation for the MultiThreadedExecutor should try assign the tasks to different threads.
@jacquelinekay jacquelinekay mentioned this issue Aug 13, 2015
Merged
@jacquelinekay jacquelinekay self-assigned this Aug 13, 2015
@jacquelinekay jacquelinekay added the ready Work is about to start (Kanban column) label Sep 10, 2015
@jacquelinekay
Copy link
Contributor Author

This relates to the upcoming discussion about the rcl API: how should we define the "spin" functions in Executor?

spin: Blocking call, do work indefinitely as it comes in.

spin_once: Do one "cycle" of work, with optional timeout. In a single-threaded executor, this means executing the first available executable. In a multi-threaded executor, does this mean executing exactly one executable, or does it mean executing one executable per thread? Does blocking mean that all threads block until they have work to do, or

spin_some: Do all the work that is immediately available to the executor. This is probably the most ill-defined of the spin functions. The current single-threaded implementation is:

  1. Check if there is available work (non-blocking)
  2. Do the available work.
  3. Repeat until there is no more available work.

A few ideas for spin_some for MultiThreadedExecutor:

  • Atomically check all of the work that is immediately available without blocking. Then, assign the executables to each available thread. If # of executables is less than # of threads, some threads might not do any work. If # of executables is greater than # of threads, each thread receives a fair amount of executables and executes them concurrently with the other threads.
    This does not allow for new work to come in while work is being done, so it is not consistent with the single-threaded implementation.
  • Naively launch N threads (where N is the hardware concurrency) where each thread runs the single-threaded spin_some. The get_next_executable call is protected with a mutex, the execute_any_executable call is not.
    The problem with this implementation is if new work comes in after threads have joined, then the new work cannot be assigned to these joined threads.

Each spin function should have a spin_node equivalent, and vice versa.

The MultiThreadedExecutor probably should be modified to have a thread pool rather than spawning and joining threads at the beginning and end of each spin call.

Also, it needs a test.

@jacquelinekay jacquelinekay added in progress Actively being worked on (Kanban column) and removed ready Work is about to start (Kanban column) labels Sep 11, 2015
@tfoote tfoote added ready Work is about to start (Kanban column) and removed in progress Actively being worked on (Kanban column) ready Work is about to start (Kanban column) labels Sep 15, 2015
@jacquelinekay jacquelinekay added the ready Work is about to start (Kanban column) label Nov 11, 2015
@jacquelinekay
Copy link
Contributor Author

there should also be multi-threaded implementations of spin_once and `spin_until_future_complete...

I'm pulling this out of the backlog because of my current work on abstracting synchronization in rclcpp.

@jacquelinekay jacquelinekay added in progress Actively being worked on (Kanban column) and removed ready Work is about to start (Kanban column) labels Nov 11, 2015
@tfoote tfoote removed the in progress Actively being worked on (Kanban column) label Nov 17, 2015
@clalancette clalancette added the enhancement New feature or request label Oct 26, 2018
gonzodepedro pushed a commit to gonzodepedro/rclcpp that referenced this issue Feb 22, 2022
@alsora
Merge pull request ros2#85 from mauropasse/mauro/add-rmw-listener-api…
4a1dc68 
…s-fix

Action unset callbacks only if they were set
nnmm pushed a commit to ApexAI/rclcpp that referenced this issue Jul 9, 2022
@mikaelarguedas
Zero init topic names and types (ros2#85)
32bcd0d 
* initialize topic_names_and_type struct in rcl

* sanity check in get_topic_names_and_types

* uncrustify

* initialize all fields...

* address documentation request

* typo
DensoADAS pushed a commit to DensoADAS/rclcpp that referenced this issue Aug 5, 2022
@Martin-Idel-SI @Karsten1987
Introduce rosbag2_transport layer and CLI (ros2#38)
1370c7f 
* rosbag2_transport package with python interface

* use cpp for python extension

* use rosbag2_transport cpp API

* use rosbag2_transport API in cli

* linters

* ros2GH-25 Rename target librosbag2 to rosbag2

CMake already prepends libraries with `lib`, so the old name resulted
in `liblibrosbag2`

* ros2GH-21 Initial call of rosbag2.record() from rosbag2_transport

* ros2GH-21 Add missing copyright header

* ros2GH-21 Cleanup clang tidy issues

* ros2GH-21 Remove rclcpp dependency from rosbag2

* ros2GH-21 Wire rosbag play into CLI

* ros2GH-21 Add missing test_depend in rosbag2_transport package.xml

* ros2GH-21 Unify name of python import

* ros2GH-21 Enable -a in CLI, show help on wrong args

* ros2GH-85 Introduce topic and type struct for readability

* ros2GH-85 Do not export sqlite3 as dependency from default plugins

- not referenced in header, therefore unnecessary

* ros2GH-85 Move rosbag2 except typesupport to rosbag2_transport

* ros2GH-85 Add rosbag2 wrapper

* ros2GH-85 Change signature of create_topic to take TopicWithType

* ros2GH-85 Use rosbag2 in rosbag2_transport

- Don't link against rosbag2_storage anymore

* ros2GH-84 Cleanup package.xmls and CMakeLists everywhere

* ros2GH-21 Add missing init() and shutdown() in record

* ros2GH-85 Fix Windows build

* ros2GH-85 Add visibility control to rosbag2

* ros2GH-85 Cleanup and documentation

* ros2GH-87 Add test package rosbag2_tests

* ros2GH-87 [WIP] Add first working prototype of an end-to-end test

* ros2GH-87 Use test_msgs instead of std_msgs/String in end-to-end test

* ros2GH-87 Use SIGTERM instead of SIGKILL and refactor test

* ros2GH-87 Make end-to-end test work on Windows

* ros2GH-87 Fix uncrustify

* ros2GH-87 Refactor end-to-end test fixture

* ros2GH-21 Extend transport python module interface

The python interface should accept all options that can be passed to rosbag2_transport

* ros2GH-87 Fix test fixture for Windows

* ros2GH-87 Refactor test fixture

* ros2GH-87 Separate record from play end-to-end test

* ros2GH-87 Make record end-to-end test work

* ros2GH-87 Publish before recording to create topic

* ros2GH-87 Fix record all on Windows

* ros2GH-87 Check for topics instead of all

* ros2GH-87 Wait until rosbag record opened database

* ros2GH-87 Delete directory recursively

* ros2GH-87 Delete directories recursively on Linux

* ros2GH-87 Reset ROS_DOMAIN_ID to protect against concurrent tests

* ros2GH-89 Make rosbag2 interfaces virtual and add explicit open() method

This allows downstream packages (e.g. rosbag2_transport) to mock these
interfaces in tests.

* ros2GH-87 Improve test and refactoring

* ros2GH-87 Minor refactoring to increase test readability

* ros2GH-87 Fix environmental variable behaviour on Mac

* ros2GH-87 Fix Windows build

* ros2GH-89 Use mock reader and writer in rosbag2_transport tests

* ros2GH-87 Add play end_to_end test

* ros2GH-87 Improvements of test

* ros2GH-87 Fix Windows build

* ros2GH-89 Cleanup: small documentation fixes.

* ros2GH-89 [WIP] Test if Writer and Reader work with class visibility

* ros2GH-87 Stabilize rosbag2_play test

* ros2GH-87 Minor refactoring of tests

* ros2GH-87 Rename end to end tests

* add license agreement

* ros2GH-89 Simplification of writing to in-memory storage

* ros2GH-89 Stabilize transport tests

* ros2GH-87 Refactoring of tests

- Extract temporary file handling
- Extract subscription management

* ros2GH-87 Add pytest cache to gitignore

* ros2GH-87 Refactoring of play test

- Extract Publisher manager

* ros2GH-87 Extract record test fixture for readability

* ros2GH-89 Refactor transport tests

- Use subscription and publisher manager just as e2e tests
- Use options in recording

* ros2GH-89 Use temporary directory fixture in sqlite tests

* ros2GH-89 Conform to naming standard for tests

* ros2GH-89 Prevent burst publishing of all messages

- Improves test stability

* ros2GH-89 Improve play stability

- Sometimes the first message is lost (discovery)

* ros2GH-25 Fix package.xmls

* Consistently use project name in CMakeLists

* Minor cleanup

- make rosbag2_transport description more expressive
- hide unnecessary methods in typesupport_helpers
- fix incorrect logging in tests
- minor cleanup

* Change name of nodes in rosbag2_transport

* Cleanup folder structure in rosbag2_storage and rosbag2_tests

- use src/<package_name>/ and test/<package_name>/ folders everywhere
- harmonises with all other packages
- results in better header guards

* Export sqlite3 dependency as package dependency

* Create node in Rosbag2Transport always

* Only hold one node in rosbag2_transport

* Move all duplicate files to common package

* Adapt namespacing in test commons package

- use "using namespace" declaratives for tests
- use package name as namespace

* Replace "Waiting for messages..." message

* ros2GH-25 rename rosbag2_test_commons -> rosbag2_test_common

* ros2GH-25 Overwrite already existing test.bag when recording

This is a temporary solution and will be handled properly once a
file path can be passed via the cli.

* ros2GH-25 Cleanups

- Log every subscription
- move all dependencies onside BUILD_TESTING for rosbag2_test_common

* fix cmake typo for test_common

* Remove superfluous loop in rosbag2 transport

* Delete superfluous test_msgs dependency

* Add rclcpp to test dependencies

- Apparently ament_export_dependencies does not work in rosbag2_test_common

* Fix rosbag2 node test

- Clock topic is no longer present on all nodes
- Remove assumptions on foreign ros topics

* Fix dependencies by exporting them explicitly
@clalancette
Copy link
Contributor

This was done ages ago, so closing this out.

@TakashiSato TakashiSato mentioned this issue Jun 1, 2024
Merged
This was referenced Jun 3, 2024
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@clalancette @tfoote @jacquelinekay