Skip to content

feat: Added new statistics interface / removed statistics publisher - #3203

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

feat: Added new statistics interface / removed statistics publisher#3203
jmachowinski wants to merge 1 commit into
rollingfrom
jm/remove_statistics_publisher

Conversation

@jmachowinski

Copy link
Copy Markdown
Collaborator

Description

This removes the half backed statistics implementation and replaces it with a interface that can be implemented by a third party pkg to archive the same functionality.

Is this user-facing behavior change?

Yes, we remove an existing feature. Due to recent bug reports I believe almost nobody was using it.
But as states, this also enables community pkgs to replicate the existing behavior.

Did you use Generative AI?

Yes, SWE-1.7

This removes the half backed statistics implementation and replaces it
with a interface that can be implemented by a third party pkg to archive
the same functionality.

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

mergify Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@jmachowinski

Copy link
Copy Markdown
Collaborator Author

Pulls: #3203
Gist: https://gist.githubusercontent.com/jmachowinski/43427d9eb5ad9b3e5aff3061c21666bc/raw/e02b86c54cc2197263e6a285fba45a2077543125/ros2.repos
BUILD args:
TEST args:
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/19897

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

@jmachowinski

Copy link
Copy Markdown
Collaborator Author

This reduced the compile time of a node with 1 sub and 1 publisher from 6.32s to 4.91s on a Ryzen 9900X

@skyegalaxy
skyegalaxy self-requested a review July 23, 2026 16:47
@skyegalaxy skyegalaxy self-assigned this Jul 23, 2026

@fujitatomoya fujitatomoya left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I am not against this approach, imo old implementation clearly wasn't pulling its weight anyway. a few comments below.

  • should we not give deprecation period before removal? statistics has been there for a long time, there would be some users who might use the statistics.
  • no default implementation provided by mainline? i think user can still do enable/disable the statistics feature on the subscription, and then mainline provides the default collector?
  • this still supports rclcpp-only, so rclpy and other client libraries can't benefit. we know that this is the same limitation the old design had. probably this is next step after this PR, for longer term I think that statistics hooks is an rcl-level event/emission API, which would give all client libraries and all entity types (publishers, services, actions) a shared mechanism, with a default implementation maintained and tested in mainline.

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.

3 participants