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

Implement lock free ring buffer with throttling [NOETIC] #319

Merged
merged 3 commits into from
May 28, 2024

Conversation

Samahu
Copy link
Contributor

@Samahu Samahu commented Apr 16, 2024

Related Issues & PRs

Problem Analysis

  • When adding multiple subscribers or when having a bad network or low performing system, LidarScan composition and processing could eventually take more time, this leads to a build up in the packets buffer which will eventually be exhausted and the current mechanism of the thread_safe_ring_buffer::write_overwrite made things only worse. The solution can be in one of two things either ensure that the LidarScan composition + processing takes less time than the time needed to add new packets, or decouple packet addition from LidarScan consumption, if the consumption thread can't keep up then throttle.

Summary of Changes

  • Implement lock free ring buffer with unit tests
  • Decouple packet reception from packet processing
  • Throttle the ring buffer as we reach 70% capacity
  • Drop packets only when the ring buffer has been completely exhausted

TODO(s): {WILL BE DONE IN A FOLLOW UP PR}

  • Replace the LockFreeRingBuffer implementation with ThreadSafeRingBuffer within os_sensor_nodelet.
    • Alternatively, forward the packets directly to LidarScan without a staging buffers.

Validation

  • Run the driver as usual with RIVZ, add multiple subscribers to /ouster/points
  • Verify the problem with missing columns is eliminated or reduced.
  • Observe that the point cloud publish frequency isn't affected significantly

@Samahu Samahu added the bug Something isn't working label Apr 16, 2024
@Samahu Samahu self-assigned this Apr 16, 2024
@Samahu Samahu added the enhancement New feature or request label Apr 16, 2024
@Samahu Samahu changed the title WIP: Implement lock free ring buffer with throttling Implement lock free ring buffer with throttling Apr 16, 2024
@Samahu Samahu changed the title Implement lock free ring buffer with throttling Implement lock free ring buffer with throttling [NOETIC] Apr 16, 2024
@Samahu Samahu requested review from twslankard and celentes May 15, 2024 17:39
@Samahu Samahu marked this pull request as ready for review May 15, 2024 17:39
@Samahu Samahu merged commit 346251b into master May 28, 2024
1 check passed
@Samahu Samahu deleted the implement_lock_free_ring_buffer_with_throttling branch May 28, 2024 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parts of point cloud missing Missing lidar chunk os1-128-u
1 participant