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

feat(ndt_scan_matcher): zero mutex blocking for ndt map update #6480

Merged

Conversation

anhnv3991
Copy link
Contributor

@anhnv3991 anhnv3991 commented Feb 22, 2024

Description

  • In map_update_module, when assigning the ndt_ptr_ to the secondary_ndt_ptr_, the old NDT pointed by ndt_ptr_ is destroyed. The cost of the destructor is significant within the mutex lock, thus lengthen the locking duration.
  • This PR adds a dummy pointer to enable the destructor to be called outside the mutex lock, thus reduces the locking duration to nearly zero.

Tests performed

    ...    
    ndt_ptr_mutex_->lock();
    // Record the begining of locking mutex
    const auto exe_start_time = std::chrono::system_clock::now();
    // End of adding
    auto dummy_ptr = ndt_ptr_;
    ...
    ndt_ptr_mutex_->unlock();

    // Record the end of locking mutex
    const auto exe_end_time = std::chrono::system_clock::now();
    const auto duration_micro_sec =
      std::chrono::duration_cast<std::chrono::microseconds>(exe_end_time - exe_start_time).count();
    const auto exe_time = static_cast<double>(duration_micro_sec) / 1000.0;
    // Output the locking duration in milliseconds
    RCLCPP_INFO(logger_, "Time duration for locking: %lf [ms]", exe_time);
    // End of adding
  • On terminal 1, launch autoware's logging simulator: ros2 launch autoware_launch logging_simulator.launch.xml \ map_path:=~/Downloads/nishishinjuku_autoware_map_divided \ vehicle_model:=sample_vehicle \ sensor_model:=awsim_sensor_kit
  • On terminal 2, replay the rosbag ros2 bag play ${ROSBAG_PATH} --rate 1.0 \ --clock 200
  • The terminal 1 should output the locking duration around 9 - 20 milliseconds on the old code, and around 0.05 milliseconds on this PR. (Tested on Intel Core i9-13900K, 64GB DDR4 2333MHz)

Effects on system behavior

None

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.

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.

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

…dt ptr

Signed-off-by: anhnv3991 <anh.nguyen.2@tier4.jp>
@github-actions github-actions bot added the component:localization Vehicle's position determination in its environment. (auto-assigned) label Feb 22, 2024
Copy link
Contributor

@SakodaShintaro SakodaShintaro left a comment

Choose a reason for hiding this comment

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

Thank you for the pull request.

The terminal 1 should output the locking duration around 9 - 20 milliseconds on the old code, and around 0.05 milliseconds on this PR. (Tested on Intel Core i9-13900K, 64GB DDR4 2333MHz)

A similar improvement was confirmed in my environment.
Great Work!

@SakodaShintaro SakodaShintaro added the tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Feb 26, 2024
Copy link

codecov bot commented Feb 26, 2024

Codecov Report

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

Project coverage is 14.72%. Comparing base (4b784b4) to head (ecaa879).
Report is 2 commits behind head on main.

Files Patch % Lines
...ization/ndt_scan_matcher/src/map_update_module.cpp 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6480      +/-   ##
==========================================
- Coverage   14.72%   14.72%   -0.01%     
==========================================
  Files        1900     1900              
  Lines      130361   130362       +1     
  Branches    38381    38381              
==========================================
  Hits        19198    19198              
- Misses      89647    89648       +1     
  Partials    21516    21516              
Flag Coverage Δ *Carryforward flag
differential 3.65% <0.00%> (?)
total 14.72% <ø> (ø) Carriedforward from 4b784b4

*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.

@anhnv3991 anhnv3991 enabled auto-merge (squash) February 27, 2024 08:37
@anhnv3991 anhnv3991 merged commit e64415b into autowarefoundation:main Feb 27, 2024
20 of 23 checks passed
@anhnv3991 anhnv3991 deleted the feature/zero_block_ndt_update branch February 28, 2024 00:12
SakodaShintaro pushed a commit to tier4/autoware.universe that referenced this pull request Feb 28, 2024
…arefoundation#6480)

Added a dummy pointer to delay the destructor of NDT when switching ndt ptr

Signed-off-by: anhnv3991 <anh.nguyen.2@tier4.jp>
HansRobo pushed a commit that referenced this pull request Mar 12, 2024
Added a dummy pointer to delay the destructor of NDT when switching ndt ptr

Signed-off-by: anhnv3991 <anh.nguyen.2@tier4.jp>
Signed-off-by: Kotaro Yoshimoto <pythagora.yoshimoto@gmail.com>
karishma1911 pushed a commit to Interplai/autoware.universe that referenced this pull request Jun 3, 2024
…arefoundation#6480)

Added a dummy pointer to delay the destructor of NDT when switching ndt ptr

Signed-off-by: anhnv3991 <anh.nguyen.2@tier4.jp>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:localization Vehicle's position determination in its environment. (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.

None yet

2 participants