Skip to content

Commit

Permalink
feat(pointcloud_preprocessor): add pipeline latency time debug inform…
Browse files Browse the repository at this point in the history
…ation for pointcloud pipeline

Signed-off-by: Berkay Karaman <[email protected]>
update
  • Loading branch information
brkay54 committed Jan 31, 2024
1 parent 132d7d8 commit cbf3c5e
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 2 deletions.
40 changes: 39 additions & 1 deletion sensing/pointcloud_preprocessor/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,45 @@ Detail description of each filter's algorithm is in the following links.

## Assumptions / Known limits

`pointcloud_preprocessor::Filter` is implemented based on pcl_perception [1] because of [this issue](https://github.com/ros-perception/perception_pcl/issues/9).
`pointcloud_preprocessor::Filter` is implemented based on pcl_perception [1] because
of [this issue](https://github.com/ros-perception/perception_pcl/issues/9).

## Measuring the performance

In Autoware, point cloud data from each LiDAR sensor undergoes preprocessing in the sensing pipeline before being input
into the perception pipeline. The preprocessing stages are illustrated in the diagram below:

![pointcloud_preprocess_pipeline.drawio.png](docs%2Fimage%2Fpointcloud_preprocess_pipeline.drawio.png)

Each stage in the pipeline incurs a processing delay. Mostly, we've used `ros2 topic delay /topic_name` to measure
the time between the message header and the current time. This approach works well for small-sized messages. However,
when dealing with large point cloud messages, this method introduces an additional delay. This is primarily because
accessing these large point cloud messages externally impacts the pipeline's performance.

Our sensing/perception nodes are designed to run within composable node containers, leveraging intra-process
communication. External subscriptions to these messages (like using ros2 topic delay or rviz2) impose extra delays and
can even slow down the pipeline by subscribing externally. Therefore, these measurements will not be accurate.

To mitigate this issue, we've adopted a method where each node in the pipeline reports its pipeline latency time.
This approach ensures the integrity of intra-process communication and provides a more accurate measure of delays in the
pipeline.

### Benchmarking The Pipeline

The nodes within the pipeline report the pipeline latency time, indicating the duration from the sensor driver's pointcloud
output to the node's output. This data is crucial for assessing the pipeline's health and efficiency.

When running Autoware, you can monitor the pipeline latency times for each node in the pipeline by subscribing to the
following ROS 2 topics:

- `/sensing/lidar/LidarX/crop_box_filter_self/debug/pipeline_latency_ms`
- `/sensing/lidar/LidarX/crop_box_filter_mirror/debug/pipeline_latency_ms`
- `/sensing/lidar/LidarX/distortion_corrector/debug/pipeline_latency_ms`
- `/sensing/lidar/LidarX/ring_outlier_filter/debug/pipeline_latency_ms`
- `/sensing/lidar/concatenate_data_synchronizer/debug/sensing/lidar/LidarX/pointcloud/pipeline_latency_ms`

These topics provide the pipeline latency times, giving insights into the delays at various stages of the pipeline
from the sensor output of LidarX to each subsequent node.

## (Optional) Error detection and handling

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,20 @@ void PointCloudConcatenateDataSynchronizerComponent::publish()
const auto & transformed_raw_points =
PointCloudConcatenateDataSynchronizerComponent::combineClouds(concat_cloud_ptr);

for (const auto & e : cloud_stdmap_) {
if (e.second != nullptr) {
if (debug_publisher_) {

Check warning on line 385 in sensing/pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_nodelet.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_nodelet.cpp#L383-L385

Added lines #L383 - L385 were not covered by tests
const auto pipeline_latency_ms =
std::chrono::duration<double, std::milli>(
std::chrono::nanoseconds(
(this->get_clock()->now() - e.second->header.stamp).nanoseconds()))
.count();
debug_publisher_->publish<tier4_debug_msgs::msg::Float64Stamped>(
"debug" + e.first + "/pipeline_latency_ms", pipeline_latency_ms);

Check warning on line 392 in sensing/pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_nodelet.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_nodelet.cpp#L389-L392

Added lines #L389 - L392 were not covered by tests
}
}
}

Check warning on line 396 in sensing/pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_nodelet.cpp

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Complex Method

PointCloudConcatenateDataSynchronizerComponent::publish has a cyclomatic complexity of 9, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.

Check warning on line 396 in sensing/pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_nodelet.cpp

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Bumpy Road Ahead

PointCloudConcatenateDataSynchronizerComponent::publish has 2 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is one single, nested block per function. The Bumpy Road code smell is a function that contains multiple chunks of nested conditional logic. The deeper the nesting and the more bumps, the lower the code health.
// publish concatenated pointcloud
if (concat_cloud_ptr) {
auto output = std::make_unique<sensor_msgs::msg::PointCloud2>(*concat_cloud_ptr);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ CropBoxFilterComponent::CropBoxFilterComponent(const rclcpp::NodeOptions & optio
using tier4_autoware_utils::DebugPublisher;
using tier4_autoware_utils::StopWatch;
stop_watch_ptr_ = std::make_unique<StopWatch<std::chrono::milliseconds>>();
debug_publisher_ = std::make_unique<DebugPublisher>(this, "crop_box_filter");
debug_publisher_ = std::make_unique<DebugPublisher>(this, this->get_name());

Check warning on line 68 in sensing/pointcloud_preprocessor/src/crop_box_filter/crop_box_filter_nodelet.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/pointcloud_preprocessor/src/crop_box_filter/crop_box_filter_nodelet.cpp#L68

Added line #L68 was not covered by tests
stop_watch_ptr_->tic("cyclic_time");
stop_watch_ptr_->tic("processing_time");
}
Expand Down Expand Up @@ -195,6 +195,14 @@ void CropBoxFilterComponent::faster_filter(
"debug/cyclic_time_ms", cyclic_time_ms);
debug_publisher_->publish<tier4_debug_msgs::msg::Float64Stamped>(
"debug/processing_time_ms", processing_time_ms);

auto pipeline_latency_ms =
std::chrono::duration<double, std::milli>(
std::chrono::nanoseconds((this->get_clock()->now() - input->header.stamp).nanoseconds()))
.count();

Check warning on line 202 in sensing/pointcloud_preprocessor/src/crop_box_filter/crop_box_filter_nodelet.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/pointcloud_preprocessor/src/crop_box_filter/crop_box_filter_nodelet.cpp#L201-L202

Added lines #L201 - L202 were not covered by tests

debug_publisher_->publish<tier4_debug_msgs::msg::Float64Stamped>(

Check warning on line 204 in sensing/pointcloud_preprocessor/src/crop_box_filter/crop_box_filter_nodelet.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/pointcloud_preprocessor/src/crop_box_filter/crop_box_filter_nodelet.cpp#L204

Added line #L204 was not covered by tests
"debug/pipeline_latency_ms", pipeline_latency_ms);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,16 @@ void DistortionCorrectorComponent::onPointCloud(PointCloud2::UniquePtr points_ms

undistortPointCloud(tf2_base_link_to_sensor, *points_msg);

if (debug_publisher_) {

Check warning on line 131 in sensing/pointcloud_preprocessor/src/distortion_corrector/distortion_corrector.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/pointcloud_preprocessor/src/distortion_corrector/distortion_corrector.cpp#L131

Added line #L131 was not covered by tests
auto pipeline_latency_ms =
std::chrono::duration<double, std::milli>(
std::chrono::nanoseconds(
(this->get_clock()->now() - points_msg->header.stamp).nanoseconds()))
.count();
debug_publisher_->publish<tier4_debug_msgs::msg::Float64Stamped>(

Check warning on line 137 in sensing/pointcloud_preprocessor/src/distortion_corrector/distortion_corrector.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/pointcloud_preprocessor/src/distortion_corrector/distortion_corrector.cpp#L135-L137

Added lines #L135 - L137 were not covered by tests
"debug/pipeline_latency_ms", pipeline_latency_ms);
}

undistorted_points_pub_->publish(std::move(points_msg));

// add processing time for debug
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,14 @@ void RingOutlierFilterComponent::faster_filter(
"debug/cyclic_time_ms", cyclic_time_ms);
debug_publisher_->publish<tier4_debug_msgs::msg::Float64Stamped>(
"debug/processing_time_ms", processing_time_ms);

auto pipeline_latency_ms =
std::chrono::duration<double, std::milli>(
std::chrono::nanoseconds((this->get_clock()->now() - input->header.stamp).nanoseconds()))
.count();

Check warning on line 211 in sensing/pointcloud_preprocessor/src/outlier_filter/ring_outlier_filter_nodelet.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/pointcloud_preprocessor/src/outlier_filter/ring_outlier_filter_nodelet.cpp#L210-L211

Added lines #L210 - L211 were not covered by tests

debug_publisher_->publish<tier4_debug_msgs::msg::Float64Stamped>(

Check warning on line 213 in sensing/pointcloud_preprocessor/src/outlier_filter/ring_outlier_filter_nodelet.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/pointcloud_preprocessor/src/outlier_filter/ring_outlier_filter_nodelet.cpp#L213

Added line #L213 was not covered by tests
"debug/pipeline_latency_ms", pipeline_latency_ms);

Check warning on line 214 in sensing/pointcloud_preprocessor/src/outlier_filter/ring_outlier_filter_nodelet.cpp

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ Getting worse: Complex Method

RingOutlierFilterComponent::faster_filter already has high cyclomatic complexity, and now it increases in Lines of Code from 123 to 129. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.
}
}

Expand Down

0 comments on commit cbf3c5e

Please sign in to comment.