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.

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

refactor(perception_rviz_plugin): only delete markers with unused MarkersIDs during updates. #4783

Merged

Conversation

Owen-Liuyuxuan
Copy link
Contributor

@Owen-Liuyuxuan Owen-Liuyuxuan commented Aug 28, 2023

Description

We have observed issues related to the processing speed of RVIZ (perception plugin) under scenarios with large amounts of detectable objects.

After profiling, we note that PredictedObjectsDisplay clears / re-adds all markers during updates with new messages, and this behavior is related to the increasing processing time.

By only deleting old marker ids not used in the newest frame, and allowing RVIZ to directly modify the old markers instead of re-adding them, this PR should improve the processing efficiency of PredictedObjectsDisplay at scenes with lots of objects.

Related links

Private Profiling Results

Tests performed

The tests are performed on the private ROSbag

Profiling Results:

image

The "update" function originally takes up (1086 + 8759 + 5006) * 1e-6 = 0.014 seconds on average. Now it takes (803 + 3217) * 1e-6 = 0.004 seconds on average. Selective Delete improved significantly on the time consumed in the update function.

Notice: The time is measured on my personal computer and does not represent the exact runtime on the deployment environment.

Notes for reviewers

Interface changes

This PR will add a public "deleteMarker" method in object_polygon_display_base.hpp.

Effects on system behavior

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

  • The PR follows the pull request guidelines.
  • The PR has been properly tested.
  • The PR has been reviewed by the code owners.

Post-review checklist for the PR author

The PR author must check the checkboxes below before merging.

  • There are no open discussions or they are tracked via tickets.
  • The PR is ready for merge.

After all checkboxes are checked, anyone who has write access can merge the PR.

…kersIDs during updates.

Signed-off-by: Owen-Liuyuxuan <uken.ryu@tier4.jp>
@github-actions github-actions bot added the component:common Common packages from the autoware-common repository. (auto-assigned) label Aug 28, 2023
@Owen-Liuyuxuan Owen-Liuyuxuan changed the title refactor(perception_rviz_plugin): only delete markers with unused Mar… refactor(perception_rviz_plugin): only delete markers with unused MarkersIDs during updagtes. Aug 28, 2023
@Owen-Liuyuxuan Owen-Liuyuxuan changed the title refactor(perception_rviz_plugin): only delete markers with unused MarkersIDs during updagtes. refactor(perception_rviz_plugin): only delete markers with unused MarkersIDs during updates. Aug 28, 2023
@shmpwk shmpwk self-requested a review August 28, 2023 08:42
Copy link
Contributor

@shmpwk shmpwk left a comment

Choose a reason for hiding this comment

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

LGTM

@shmpwk shmpwk enabled auto-merge (squash) August 28, 2023 23:16
@shmpwk shmpwk added the tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Aug 28, 2023
@shmpwk
Copy link
Contributor

shmpwk commented Aug 28, 2023

@taikitanaka3 @1222-takeshi Could you approve this as a codeowner?

Copy link
Contributor

@taikitanaka3 taikitanaka3 left a comment

Choose a reason for hiding this comment

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

lgtm

@codecov
Copy link

codecov bot commented Aug 29, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (e017725) 15.09% compared to head (964b418) 15.09%.
Report is 15 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4783   +/-   ##
=======================================
  Coverage   15.09%   15.09%           
=======================================
  Files        1568     1568           
  Lines      108130   108128    -2     
  Branches    33173    33165    -8     
=======================================
  Hits        16321    16321           
- Misses      74031    74037    +6     
+ Partials    17778    17770    -8     
Flag Coverage Δ *Carryforward flag
differential 0.00% <0.00%> (?)
total 15.09% <ø> (+<0.01%) ⬆️ Carriedforward from e017725

*This pull request uses carry forward flags. Click here to find out more.

Files Changed Coverage Δ
...e/object_detection/object_polygon_display_base.hpp 0.00% <0.00%> (ø)
...ude/object_detection/predicted_objects_display.hpp 0.00% <ø> (ø)
...src/object_detection/predicted_objects_display.cpp 0.00% <0.00%> (ø)
...ior_velocity_detection_area_module/src/manager.cpp 52.00% <ø> (+12.60%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@shmpwk shmpwk merged commit 0cbfc3d into autowarefoundation:main Aug 29, 2023
30 of 32 checks passed
@Owen-Liuyuxuan Owen-Liuyuxuan deleted the refactor/perception_rviz_plugin branch August 29, 2023 01:43
shmpwk pushed a commit to tier4/autoware.universe that referenced this pull request Aug 31, 2023
…kersIDs during updates. (autowarefoundation#4783)

* refactor(perception_rviz_plugin): only delete markers with unused MarkersIDs during updates.

Signed-off-by: Owen-Liuyuxuan <uken.ryu@tier4.jp>

* style(pre-commit): autofix

* fix: include <set> for linting test

Signed-off-by: Owen-Liuyuxuan <uken.ryu@tier4.jp>

* style(pre-commit): autofix

---------

Signed-off-by: Owen-Liuyuxuan <uken.ryu@tier4.jp>
Co-authored-by: Owen-Liuyuxuan <uken.ryu@tier4.jp>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
s-azumi pushed a commit to tier4/autoware.universe that referenced this pull request Feb 28, 2024
…kersIDs during updates. (autowarefoundation#4783)

* refactor(perception_rviz_plugin): only delete markers with unused MarkersIDs during updates.

Signed-off-by: Owen-Liuyuxuan <uken.ryu@tier4.jp>

* style(pre-commit): autofix

* fix: include <set> for linting test

Signed-off-by: Owen-Liuyuxuan <uken.ryu@tier4.jp>

* style(pre-commit): autofix

---------

Signed-off-by: Owen-Liuyuxuan <uken.ryu@tier4.jp>
Co-authored-by: Owen-Liuyuxuan <uken.ryu@tier4.jp>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:common Common packages from the autoware-common repository. (auto-assigned) tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants