Skip to content

feat(EventsCBGExecutor): Added worker queue - #3277

Open
jmachowinski wants to merge 1 commit into
rollingfrom
jm/cbg_worker_queue
Open

jmachowinski wants to merge 1 commit into
rollingfrom
jm/cbg_worker_queue

Conversation

@jmachowinski

Copy link
Copy Markdown
Collaborator

Description

The worker queue should improve the usage of cold vs hot threads, by reusing threads that have been recently used.

There are also some improvements around not waking up threads if they can't get work anyway.

Is this user-facing behavior change?

No

Did you use Generative AI?

Yes for code reviews and checks

Additional Information

The worker queue should improve the usage of cold vs hot threads,
by reusing threads that have been recently used.

There are also some improvements around not waking up threads if
they can't get work anyway.

Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
@jmachowinski

Copy link
Copy Markdown
Collaborator Author

Pulls: #3277
Gist: https://gist.githubusercontent.com/jmachowinski/fa6541200506e08495142fae71d2d6b2/raw/751aaa1a4d154e4dee96b231bbc5e488917bb539/ros2.repos
BUILD args:
TEST args:
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/20486

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@github-actions

Copy link
Copy Markdown

ABI Compliance Check

Verdict: incompatible

Library Verdict Summary
libcomponent_manager.so ✅ compatible No ABI changes detected.
librclcpp.so ❌ incompatible ABI-incompatible changes detected.
librclcpp_action.so ✅ compatible No ABI changes detected.
librclcpp_lifecycle.so ✅ compatible No ABI changes detected.
libcomponent_manager.so — full abidiff report

Compared:

  • Base: lib-base/libcomponent_manager.so
  • Head: lib-pr/libcomponent_manager.so @ 9880d0b
(empty report — no differences printed by abidiff)
librclcpp.so — full abidiff report

Compared:

  • Base: lib-base/librclcpp.so
  • Head: lib-pr/librclcpp.so @ 9880d0b
Functions changes summary: 2 Removed (3 filtered out), 2 Changed (134 filtered out), 9 Added (46 filtered out) functions
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
Function symbols changes summary: 2 Removed, 4 Added function symbols not referenced by debug info
Variable symbols changes summary: 0 Removed, 0 Added variable symbol not referenced by debug info

2 Removed functions:

  [D] 'method void rclcpp::executors::cbg_executor::CBGScheduler::block_worker_thread()'    {_ZN6rclcpp9executors12cbg_executor12CBGScheduler19block_worker_threadEv}
  [D] 'method void rclcpp::executors::cbg_executor::CBGScheduler::block_worker_thread_for(std::chrono::nanoseconds)'    {_ZN6rclcpp9executors12cbg_executor12CBGScheduler23block_worker_thread_forENSt6chrono8durationIlSt5ratioILl1ELl1000000000EEEE}

9 Added functions:

  [A] 'method void rclcpp::executors::cbg_executor::CBGScheduler::block_worker_thread(rclcpp::executors::cbg_executor::Worker*)'    {_ZN6rclcpp9executors12cbg_executor12CBGScheduler19block_worker_threadEPNS1_6WorkerE}
  [A] 'method void rclcpp::executors::cbg_executor::CBGScheduler::block_worker_thread_for(rclcpp::executors::cbg_executor::Worker*, std::chrono::nanoseconds)'    {_ZN6rclcpp9executors12cbg_executor12CBGScheduler23block_worker_thread_forEPNS1_6WorkerENSt6chrono8durationIlSt5ratioILl1ELl1000000000EEEE}
  [A] 'method bool rclcpp::executors::cbg_executor::CBGScheduler::prepare_and_enqueue_worker(rclcpp::executors::cbg_executor::Worker*)'    {_ZN6rclcpp9executors12cbg_executor12CBGScheduler26prepare_and_enqueue_workerEPNS1_6WorkerE}
  [A] 'method void rclcpp::executors::cbg_executor::CBGScheduler::suppress_thread_wakeup()'    {_ZN6rclcpp9executors12cbg_executor12CBGScheduler22suppress_thread_wakeupEv}
  [A] 'method void rclcpp::executors::cbg_executor::Worker::block()'    {_ZN6rclcpp9executors12cbg_executor6Worker5blockEv}
  [A] 'method void rclcpp::executors::cbg_executor::Worker::block_for(std::chrono::nanoseconds)'    {_ZN6rclcpp9executors12cbg_executor6Worker9block_forENSt6chrono8durationIlSt5ratioILl1ELl1000000000EEEE}
  [A] 'method void rclcpp::executors::cbg_executor::Worker::unblock()'    {_ZN6rclcpp9executors12cbg_executor6Worker7unblockEv}
  [A] 'method rclcpp::executors::cbg_executor::Worker* rclcpp::executors::cbg_executor::WorkerQueue::pop_blocked_worker_thread()'    {_ZN6rclcpp9executors12cbg_executor11WorkerQueue25pop_blocked_worker_threadEv}
  [A] 'method void rclcpp::executors::cbg_executor::WorkerQueue::remove_worker_thread(rclcpp::executors::cbg_executor::Worker*)'    {_ZN6rclcpp9executors12cbg_executor11WorkerQueue20remove_worker_threadEPNS1_6WorkerE}

2 functions with some indirect sub-type change:

  [C] 'method rclcpp::executors::cbg_executor::CBGScheduler::CBGScheduler(std::function<void()>)' at scheduler.hpp:195:1 has some indirect sub-type changes:
    implicit parameter 0 of type 'rclcpp::executors::cbg_executor::CBGScheduler*' has sub-type changes:
      in pointed to type 'class rclcpp::executors::cbg_executor::CBGScheduler' at scheduler.hpp:142:1:
        type size changed from 1984 to 2624 (in bits)
        1 member function insertion:
          'method virtual rclcpp::executors::cbg_executor::CBGScheduler::~CBGScheduler()' at scheduler.hpp:309:1
        no member function changes (6 filtered);
        2 data member deletions:
          'bool release_workers', at offset 1344 (in bits) at scheduler.hpp:393:1
          'bool release_worker_once', at offset 1352 (in bits) at scheduler.hpp:394:1
        3 data member changes (1 filtered):
          name of 'rclcpp::executors::cbg_executor::CBGScheduler::work_ready_conditional' changed to 'rclcpp::executors::cbg_executor::CBGScheduler::worker_queue' at scheduler.hpp:548:1, size changed from 384 to 1024 (in bits) (by +640 bits)
          'bool worker_checking_for_work' offset changed from 1360 to 1344 (in bits) (by -16 bits)
          'std::__cxx11::list<std::unique_ptr<rclcpp::executors::cbg_executor::CBGScheduler::CallbackGroupHandle, std::default_delete<rclcpp::executors::cbg_executor::CBGScheduler::CallbackGroupHandle> >, std::allocator<std::unique_ptr<rclcpp::executors::cbg_executor::CBGScheduler::CallbackGroupHandle, std::default_delete<rclcpp::executors::cbg_executor::CBGScheduler::CallbackGroupHandle> > > > callback_groups' offset changed from 1792 to 2432 (in bits) (by +640 bits)

  [C] 'method virtual std::unique_ptr<rclcpp::executors::cbg_executor::CBGScheduler::CallbackGroupHandle, std::default_delete<rclcpp::executors::cbg_executor::CBGScheduler::CallbackGroupHandle> > rclcpp::executors::cbg_executor::FirstInFirstOutScheduler::get_handle_for_callback_group(const rclcpp::CallbackGroup::SharedPtr&)' at first_in_first_out_scheduler.cpp:149:1 has some indirect sub-type changes:
    implicit parameter 0 of type 'rclcpp::executors::cbg_executor::FirstInFirstOutScheduler*' has sub-type changes:
      in pointed to type 'class rclcpp::executors::cbg_executor::FirstInFirstOutScheduler' at first_in_first_out_scheduler.hpp:66:1:
        type size changed from 2176 to 2816 (in bits)
        1 base class change:
          'class rclcpp::executors::cbg_executor::CBGScheduler' at scheduler.hpp:32:1 changed:
            details were reported earlier
        no member function changes (3 filtered);
        1 data member change:
          'std::vector<std::unique_ptr<rclcpp::executors::cbg_executor::FirstInFirstOutCallbackGroupHandle, std::default_delete<rclcpp::executors::cbg_executor::FirstInFirstOutCallbackGroupHandle> >, std::allocator<std::unique_ptr<rclcpp::executors::cbg_executor::FirstInFirstOutCallbackGroupHandle, std::default_delete<rclcpp::executors::cbg_executor::FirstInFirstOutCallbackGroupHandle> > > > callback_group_handles' offset changed from 1984 to 2624 (in bits) (by +640 bits)

2 Removed function symbols not referenced by debug info:

  [D] _ZZN6rclcpp9executors12cbg_executor12CBGScheduler19block_worker_threadEvENKUlvE_clEv
  [D] _ZZN6rclcpp9executors12cbg_executor12CBGScheduler23block_worker_thread_forENSt6chrono8durationIlSt5ratioILl1ELl1000000000EEEEENKUlvE_clEv

4 Added function symbols not referenced by debug info:

  [A] _ZN6rclcpp9executors12cbg_executor11WorkerQueueC1Ev
  [A] _ZN6rclcpp9executors12cbg_executor11WorkerQueueC2Ev, aliases _ZN6rclcpp9executors12cbg_executor11WorkerQueueC1Ev
  [A] _ZN6rclcpp9executors12cbg_executor6WorkerC1Ev, aliases _ZN6rclcpp9executors12cbg_executor6WorkerC2Ev
  [A] _ZN6rclcpp9executors12cbg_executor6WorkerC2Ev


librclcpp_action.so — full abidiff report

Compared:

  • Base: lib-base/librclcpp_action.so
  • Head: lib-pr/librclcpp_action.so @ 9880d0b
(empty report — no differences printed by abidiff)
librclcpp_lifecycle.so — full abidiff report

Compared:

  • Base: lib-base/librclcpp_lifecycle.so
  • Head: lib-pr/librclcpp_lifecycle.so @ 9880d0b
(empty report — no differences printed by abidiff)

Updated for commit 9880d0b · suppressions: /home/runner/work/_temp/ros2-abi-suppressions.txt

@jmachowinski

jmachowinski commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator Author

@fujitatomoya this is clearly a false positive in the abi checker. It fails to realize that everything that changed is part of a pimple pattern and not public ABI.

@jmachowinski

Copy link
Copy Markdown
Collaborator Author

Also why are we running it for rolling ?

@skyegalaxy

Copy link
Copy Markdown
Member

@jmachowinski - I think it's so we can know if a change to rolling will successfully backport to older distros. but yeah this should probably also take into account PIMPL and whether an affected class is part of the public API or not

skyegalaxy
skyegalaxy approved these changes Sep 18, 2026
@skyegalaxy

Copy link
Copy Markdown
Member

quick benchmarks:
compare_cpu__cbg_study_arr_g1
compare_cpu__cbg_study_arr_g5
compare_cpu__cbg_study_arr_g10
compare_cpu__cbg_study_arr_g50
compare_latency__cbg_study_arr_g1
compare_latency__cbg_study_arr_g5
compare_latency__cbg_study_arr_g10
compare_latency__cbg_study_arr_g50
compare_rss__cbg_study_arr_g1
compare_rss__cbg_study_arr_g5
compare_rss__cbg_study_arr_g10
compare_rss__cbg_study_arr_g50

@jmachowinski

Copy link
Copy Markdown
Collaborator Author

Hu what is going on with these benchmarks, they don't match my expectations and tests at all....

@skyegalaxy

Copy link
Copy Markdown
Member

ah wait I think this was with irobot benchmark before all of our recent changes. Won't be able to rerun them for a few days

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants