Skip to content

Conversation

@bjin2364
Copy link

Background

RealtimePublisher class uses locking mechanisms to publish data in a separate thread, but given it follows a single-producer single-consumer paradigm, we can create a wait-free implementation using the LockFreeQueue data structure in this repo. This PR is an attempt to introduce this implementation.

Changes

  • Adds a WaitFreeRealtimePublisher class, which compared to RealtimePublisher:
    • similarities:
      • spawns a separate thread for publishing messages
    • differences:
      • wait-free
      • background thread has configurable priority and CPU affinity (Linux only)
      • does not spawn publishing thread on construction
      • slightly different public API
  • Introduces a benchmark script to compare WaitFreeRealtimePublisher to RealtimePublisher
    • consequently slightly changes RealtimePublisher to support benchmarking, and also adds publisher interface abstraction for benchmarks and test

Testing

  • unit tests
  • benchmark

Benchmark Results

Settings:

  • linux machine with isolated cores 0,1. Publishing thread is pinned to core 1 while main thread is core 0
  • disabled cpu scaling
  • compiled with release mode

Example Results: wait-free publishing side is 2 orders of magnitude faster than RealtimePublisher on the producer-side. I'm still a bit confused by the publishing thread side: the other day I was also seeing an order of magnitude of messages that were published, so maybe this benchmark needs some tuning.

Running ./build/realtime_tools/realtime_publishers_benchmark
Run on (12 X 400 MHz CPU s)
CPU Caches:
  L1 Data 48 KiB (x6)
  L1 Instruction 32 KiB (x6)
  L2 Unified 1280 KiB (x6)
  L3 Unified 12288 KiB (x1)
Load Average: 0.62, 0.55, 0.60
----------------------------------------------------------------------------------------------
Benchmark                                    Time             CPU   Iterations UserCounters...
----------------------------------------------------------------------------------------------
BM_RealtimePublisher                      26.9 ns         17.0 ns     41174747 num_publishes=484.333k
BM_WaitFreeRealtimePublisher<1>/1        0.398 ns        0.398 ns   1000000000 num_publishes=7.508k
BM_WaitFreeRealtimePublisher<1>/5        0.397 ns        0.397 ns   1000000000 num_publishes=6.984k
BM_WaitFreeRealtimePublisher<1>/10       0.398 ns        0.398 ns   1000000000 num_publishes=6.399k
BM_WaitFreeRealtimePublisher<1>/50       0.396 ns        0.396 ns   1000000000 num_publishes=3.866k
BM_WaitFreeRealtimePublisher<2>/1        0.340 ns        0.340 ns   1000000000 num_publishes=12.859k
BM_WaitFreeRealtimePublisher<2>/5        0.340 ns        0.340 ns   1000000000 num_publishes=11.842k
BM_WaitFreeRealtimePublisher<2>/10       0.339 ns        0.339 ns   1000000000 num_publishes=10.871k
BM_WaitFreeRealtimePublisher<2>/50       0.337 ns        0.337 ns   1000000000 num_publishes=6.621k
BM_WaitFreeRealtimePublisher<5>/1        0.314 ns        0.314 ns   1000000000 num_publishes=37.859k
BM_WaitFreeRealtimePublisher<5>/5        0.313 ns        0.313 ns   1000000000 num_publishes=36.552k
BM_WaitFreeRealtimePublisher<5>/10       0.313 ns        0.313 ns   1000000000 num_publishes=33.087k
BM_WaitFreeRealtimePublisher<5>/50       0.312 ns        0.312 ns   1000000000 num_publishes=19.569k
BM_DefaultWaitFreeRealtimePublisher      0.340 ns        0.340 ns   1000000000 num_publishes=12.854k

Notes

  • I'm not super familiar with copyright side, so just copy and pasted the same licensing as some other files in this repo.

@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 65.68047% with 58 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.84%. Comparing base (69bd9a3) to head (ad71860).

Files with missing lines Patch % Lines
..._tools/benchmark/realtime_publishers_benchmark.cpp 0.00% 27 Missing ⚠️
...de/realtime_tools/wait_free_realtime_publisher.hpp 54.54% 18 Missing and 7 partials ⚠️
realtime_tools/test/wait_free_publisher_test.cpp 92.77% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #461      +/-   ##
==========================================
- Coverage   85.25%   82.84%   -2.41%     
==========================================
  Files          17       21       +4     
  Lines        1397     1568     +171     
  Branches      132      150      +18     
==========================================
+ Hits         1191     1299     +108     
- Misses        120      173      +53     
- Partials       86       96      +10     
Flag Coverage Δ
unittests 82.84% <65.68%> (-2.41%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ools/include/realtime_tools/realtime_publisher.hpp 78.26% <100.00%> (-11.99%) ⬇️
...clude/realtime_tools/utils/publisher_interface.hpp 100.00% <100.00%> (ø)
realtime_tools/test/wait_free_publisher_test.cpp 92.77% <92.77%> (ø)
...de/realtime_tools/wait_free_realtime_publisher.hpp 54.54% <54.54%> (ø)
..._tools/benchmark/realtime_publishers_benchmark.cpp 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@christophfroehlich
Copy link
Member

Is this ready for review? We can first discuss if the approach is fine, and then try to fix the backwards compatibility and windows build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants