perf(common_ground_filter): performance tuning #5861

taisa1 · 2023-12-13T09:04:23Z

Description

This PR makes scan_ground_filter (common_ground_filter) faster without changing the logical output.
The tail latency of the scan_ground_filter gets about x1.7 faster with this PR merged.

The main changes are as follows:

Minimized the use of trigonometric functions such as std::atan2,
Processed the input point cloud without converting it to pcl::PointCloud.

In addition, some small changes are made to improve performance.

Measurement Condition

・Ubuntu22.04 + ROS2 Humble + Autoware Universe rosbag simulation
・Core Isolated
・Core Frequency Fixed (2.3GHz)
・L3 Cache: 24MB

Tests performed

It has been verified that the content of output topics with the same timestamp is identical before and after the PR.

To reproduce

Download files from https://drive.google.com/drive/folders/1703DlE8MT0rksiQfZGR8A53wwrvMRSEc?usp=drive_link and run python3 validator.py default.csv after.csv for comparing results.

Pre-review checklist for the PR author

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

I've confirmed the contribution guidelines.
The PR follows the pull request guidelines.

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.

Signed-off-by: taisa1 <[email protected]>

atsushi421

It would be great if this PR was modified to pass CodeScene.

perception/ground_segmentation/include/ground_segmentation/scan_ground_filter_nodelet.hpp

perception/ground_segmentation/src/scan_ground_filter_nodelet.cpp

Signed-off-by: taisa1 <[email protected]>

atsushi421 · 2023-12-27T23:32:17Z

@taisa1
The unit test is failed due to changes in filter(). It is recommended that the unit tests also use faster_filter().
https://github.com/autowarefoundation/autoware.universe/actions/runs/7342510285/job/19991834839?pr=5861

miursh · 2023-12-29T08:54:20Z

This is the result of TIER IV CI test. TIER IV internal link

After running our CI tests, we've observed a significant increase in the number of failure. The number of passing test cases, dropping from 148 to 68. While I haven't confirmed the details yet, it seems that some errors may be occurring.

Signed-off-by: taisa1 <[email protected]>

taisa1 · 2024-01-05T07:46:08Z

I found that some variables I added have to be updated when corresponding parameter changes, but I hadn't implemented it. This might cause failure so I fixed it.

codecov · 2024-01-08T12:02:56Z

Codecov Report

Attention: Patch coverage is 48.12500% with 83 lines in your changes are missing coverage. Please review.

Project coverage is 14.79%. Comparing base (ebb4172) to head (74fc93d).
Report is 2 commits behind head on main.

Files	Patch %	Lines
...nd_segmentation/src/scan_ground_filter_nodelet.cpp	48.40%	55 Missing and 26 partials ⚠️
...ound_segmentation/test/test_scan_ground_filter.cpp	0.00%	0 Missing and 1 partial ⚠️
sensing/pointcloud_preprocessor/src/filter.cpp	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5861      +/-   ##
==========================================
+ Coverage   14.78%   14.79%   +0.01%     
==========================================
  Files        1917     1917              
  Lines      132046   132109      +63     
  Branches    39228    39252      +24     
==========================================
+ Hits        19523    19548      +25     
- Misses      90734    90766      +32     
- Partials    21789    21795       +6

Flag	Coverage Δ		*Carryforward flag
differential	`5.52% <48.12%> (?)`
total	`14.75% <ø> (-0.04%)`	⬇️	Carriedforward from ebb4172

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

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

miursh · 2024-01-09T02:28:16Z

Thanks!
I re-ran the tests, and I can see some changes, but there are still many failures.
https://evaluation.tier4.jp/evaluation/reports/d9fcb22f-095a-5b13-89a2-82b72be71201?project_id=prd_jt

Signed-off-by: atsushi421 <[email protected]>

…r-scan_ground_filter

atsushi421 · 2024-02-05T00:22:31Z

@miursh
Sorry for the late response. 🙇
I found a variable uninitialized and have fixed it, could you review this again?

atsushi421 · 2024-03-04T06:56:45Z

@badai-nguyen
Thank you for the other day.
I used the tutorial to observe the state of the point cloud before and after PR and found no particular difference, as shown in the images below. (Used autoware.universe commit ID: ebb4172)
It seems to be working correctly in my environment, could you please review again?

Before	After

badai-nguyen

@taisa1 @atsushi421 I deeply appreciate your improvement.

I confirmed by logging_simulator that the obstacle_segmentation/pointcloud as before and the processing time of /perception/obstacle_segmentation/common_ground_filter node is improved.

Before	After

CI tested result also shows the acceptable pass rate.

LGTM!

* perf(scan_ground_filter): performance tuning Signed-off-by: taisa1 <[email protected]> * perf(scan_ground_filter): improved performance Signed-off-by: taisa1 <[email protected]> * chore: refactoring Signed-off-by: taisa1 <[email protected]> * chore: refactoring Signed-off-by: taisa1 <[email protected]> * refactor: reflect review Signed-off-by: taisa1 <[email protected]> * style(pre-commit): autofix * fix: remove unnecessary comment Signed-off-by: taisa1 <[email protected]> * test: change unit test to use faster_filter Signed-off-by: taisa1 <[email protected]> * test: fixed compile error Signed-off-by: taisa1 <[email protected]> * fix: add lacking parameter callback Signed-off-by: taisa1 <[email protected]> * fix: uninitialized valuabe Signed-off-by: atsushi421 <[email protected]> --------- Signed-off-by: taisa1 <[email protected]> Signed-off-by: atsushi421 <[email protected]> Co-authored-by: taisa1 <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: atsushi yano <[email protected]> Co-authored-by: atsushi421 <[email protected]> Signed-off-by: Kotaro Yoshimoto <[email protected]>

* perf(scan_ground_filter): performance tuning Signed-off-by: taisa1 <[email protected]> * perf(scan_ground_filter): improved performance Signed-off-by: taisa1 <[email protected]> * chore: refactoring Signed-off-by: taisa1 <[email protected]> * chore: refactoring Signed-off-by: taisa1 <[email protected]> * refactor: reflect review Signed-off-by: taisa1 <[email protected]> * style(pre-commit): autofix * fix: remove unnecessary comment Signed-off-by: taisa1 <[email protected]> * test: change unit test to use faster_filter Signed-off-by: taisa1 <[email protected]> * test: fixed compile error Signed-off-by: taisa1 <[email protected]> * fix: add lacking parameter callback Signed-off-by: taisa1 <[email protected]> * fix: uninitialized valuabe Signed-off-by: atsushi421 <[email protected]> --------- Signed-off-by: taisa1 <[email protected]> Signed-off-by: atsushi421 <[email protected]> Co-authored-by: taisa1 <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: atsushi yano <[email protected]> Co-authored-by: atsushi421 <[email protected]> Signed-off-by: kaigohirao <[email protected]>

* perf(scan_ground_filter): performance tuning Signed-off-by: taisa1 <[email protected]> * perf(scan_ground_filter): improved performance Signed-off-by: taisa1 <[email protected]> * chore: refactoring Signed-off-by: taisa1 <[email protected]> * chore: refactoring Signed-off-by: taisa1 <[email protected]> * refactor: reflect review Signed-off-by: taisa1 <[email protected]> * style(pre-commit): autofix * fix: remove unnecessary comment Signed-off-by: taisa1 <[email protected]> * test: change unit test to use faster_filter Signed-off-by: taisa1 <[email protected]> * test: fixed compile error Signed-off-by: taisa1 <[email protected]> * fix: add lacking parameter callback Signed-off-by: taisa1 <[email protected]> * fix: uninitialized valuabe Signed-off-by: atsushi421 <[email protected]> --------- Signed-off-by: taisa1 <[email protected]> Signed-off-by: atsushi421 <[email protected]> Co-authored-by: taisa1 <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: atsushi yano <[email protected]> Co-authored-by: atsushi421 <[email protected]>

taisa1 added 2 commits November 15, 2023 14:21

perf(scan_ground_filter): performance tuning

7dc808d

Signed-off-by: taisa1 <[email protected]>

perf(scan_ground_filter): improved performance

3910c3c

Signed-off-by: taisa1 <[email protected]>

github-actions bot added component:perception Advanced sensor data processing and environment understanding. (auto-assigned) component:sensing Data acquisition from sensors, drivers, preprocessing. (auto-assigned) labels Dec 13, 2023

atsushi421 self-requested a review December 22, 2023 03:56

taisa1 added 2 commits December 22, 2023 16:34

chore: refactoring

33c948f

Signed-off-by: taisa1 <[email protected]>

chore: refactoring

34e984e

Signed-off-by: taisa1 <[email protected]>

taisa1 marked this pull request as ready for review December 22, 2023 10:14

taisa1 requested review from amc-nu, miursh, yukkysaito and badai-nguyen as code owners December 22, 2023 10:14

taisa1 marked this pull request as draft December 23, 2023 11:55

atsushi421 requested changes Dec 25, 2023

View reviewed changes

taisa1 and others added 2 commits December 27, 2023 15:48

refactor: reflect review

4579d4a

Signed-off-by: taisa1 <[email protected]>

style(pre-commit): autofix

701a709

atsushi421 added the run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Dec 27, 2023

fix: remove unnecessary comment

f14627e

Signed-off-by: taisa1 <[email protected]>

atsushi421 self-requested a review December 27, 2023 07:15

This comment was marked as outdated.

Sign in to view

taisa1 marked this pull request as ready for review December 27, 2023 07:16

taisa1 changed the title ~~perf(scan_ground_filter): performance tuning~~ perf(common_ground_filter): performance tuning Dec 27, 2023

Merge branch 'main' into performance-tuning-for-scan_ground_filter

97e6370

taisa1 added 3 commits January 5, 2024 14:40

test: change unit test to use faster_filter

d3ca499

Signed-off-by: taisa1 <[email protected]>

test: fixed compile error

5dbd0f1

Signed-off-by: taisa1 <[email protected]>

fix: add lacking parameter callback

0be0c01

Signed-off-by: taisa1 <[email protected]>

atsushi421 added 2 commits February 4, 2024 12:19

fix: uninitialized valuabe

cfe05cc

Signed-off-by: atsushi421 <[email protected]>

Merge remote-tracking branch 'origin/main' into performance-tuning-fo…

89d51f8

…r-scan_ground_filter

atsushi421 marked this pull request as draft March 4, 2024 04:53

atsushi421 removed the run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Mar 4, 2024

Merge branch 'main' into performance-tuning-for-scan_ground_filter

74fc93d

atsushi421 marked this pull request as ready for review March 4, 2024 06:56

atsushi421 requested a review from kyoichi-sugahara as a code owner March 4, 2024 06:56

atsushi421 added the run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Mar 4, 2024

badai-nguyen approved these changes Mar 7, 2024

View reviewed changes

atsushi421 merged commit efe4e16 into autowarefoundation:main Mar 11, 2024
35 of 36 checks passed

atsushi421 deleted the performance-tuning-for-scan_ground_filter branch March 11, 2024 08:14

badai-nguyen added the type:performance Software optimization and system performance. label Mar 29, 2024

badai-nguyen mentioned this pull request Apr 10, 2024

chore(ground_segmentation): cherry pick pr6209+5861+6753+5980 tier4/autoware.universe#1237

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf(common_ground_filter): performance tuning #5861

perf(common_ground_filter): performance tuning #5861

taisa1 commented Dec 13, 2023 •

edited by atsushi421

Loading

atsushi421 left a comment

This comment was marked as outdated.

atsushi421 commented Dec 27, 2023

miursh commented Dec 29, 2023 •

edited

Loading

taisa1 commented Jan 5, 2024

codecov bot commented Jan 8, 2024 •

edited

Loading

miursh commented Jan 9, 2024 •

edited

Loading

atsushi421 commented Feb 5, 2024

atsushi421 commented Mar 4, 2024

badai-nguyen left a comment

perf(common_ground_filter): performance tuning #5861

perf(common_ground_filter): performance tuning #5861

Conversation

taisa1 commented Dec 13, 2023 • edited by atsushi421 Loading

Description

Measurement Condition

Related links

Tests performed

To reproduce

Pre-review checklist for the PR author

In-review checklist for the PR reviewers

Post-review checklist for the PR author

atsushi421 left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

atsushi421 commented Dec 27, 2023

miursh commented Dec 29, 2023 • edited Loading

taisa1 commented Jan 5, 2024

codecov bot commented Jan 8, 2024 • edited Loading

Codecov Report

miursh commented Jan 9, 2024 • edited Loading

atsushi421 commented Feb 5, 2024

atsushi421 commented Mar 4, 2024

badai-nguyen left a comment

Choose a reason for hiding this comment

taisa1 commented Dec 13, 2023 •

edited by atsushi421

Loading

miursh commented Dec 29, 2023 •

edited

Loading

codecov bot commented Jan 8, 2024 •

edited

Loading

miursh commented Jan 9, 2024 •

edited

Loading