Note: This is an AI summary from the benchmarking files

Benchmark Comparison Summary

Test Environments

Dev Machine (ubuntu@dexory)

• CPU: 16 cores × 5400 MHz
• OS: Ubuntu 24.04.2 LTS
• L1 Data Cache: 48 KiB × 16 = 768 KiB
• L1 Instruction Cache: 64 KiB × 16 = 1024 KiB
• L2 Cache: 3072 KiB × 16 = 48 MB (unified)
• L3 Cache: 24576 KiB = 24 MB (shared)
• Total Cache: ~74 MB

Robot (arri-74)

• CPU: 16 cores × 5000 MHz
• L1 Data Cache: 48 KiB × 8 = 384 KiB
• L1 Instruction Cache: 32 KiB × 8 = 256 KiB
• L2 Cache: 1280 KiB × 8 = 10 MB (unified)
• L3 Cache: 18432 KiB = 18 MB (shared)
• Total Cache: ~28.6 MB

Performance Comparison (Key Benchmarks)

1000×1000 Grid (1M cells, 50% occupancy, 2m inflation radius)

2000×2000 Grid (4M cells, 50% occupancy, 2m inflation radius)

3333×3333 Grid (11.1M cells, 50% occupancy, 2m inflation radius)

4000×4000 Grid (16M cells, 30% occupancy, 1m inflation radius)

Key Findings

1. New Implementation Impact (OpenMP disabled)

• Dev machine: 2.7-3.5× faster than old implementation
• Robot: 1.1-2.6× faster than old implementation
• Performance scales better on more powerful dev machine

2. OpenMP Parallelization Impact

• Dev machine: 2.5-4.8× additional speedup over single-threaded new implementation
• Robot: 1.8-5.6× additional speedup over single-threaded new implementation
• Combined with new implementation: 6.5-15.3× faster than old code

3. Grid Size Scaling

• Old implementation shows poor scaling with grid size (35-43 M cells/s)
• New implementation (OpenMP disabled) maintains 90-145 M cells/s
• New implementation (OpenMP enabled) maintains 395-893 M cells/s on dev, 364-483 M cells/s on robot

4. Occupancy Impact (1500×1500 tests)

• All implementations show relatively consistent performance across 10%, 30%, 50%, 80% occupancy
• New implementation handles varying occupancy much more efficiently

5. Inflation Radius Impact

• Old implementation: significant slowdown with larger radii (41→35→31 M cells/s)
• New implementation: minimal impact from radius variation

Detailed Results by Parameter

Varying Occupancy (1500×1500 grid, 2m inflation)

Key Observation: Old implementation degrades significantly with higher occupancy (8→75 ms on dev), while new implementation remains stable (14-16 ms without OpenMP, 4-5 ms with OpenMP).

Varying Inflation Radius (1000×1000 grid, 50% occupancy)

Key Observation: Old implementation shows 36% slowdown from smallest to largest radius (11.2→17.6 ms on dev). New implementation shows minimal variation (<3% difference).

Varying Cost Scale (1000×1000 grid, 50% occupancy, 2m radius)

Key Observation: Cost scale factor has negligible impact on performance across all implementations.

Recommendations

✅ Use new implementation with OpenMP enabled - Provides 6.5-15.3× speedup

✅ Even without OpenMP, new implementation is 1.1-3.5× faster

✅ Performance is more predictable and scales better with grid size

✅ Robot shows excellent speedup despite lower CPU frequency

✅ New implementation handles varying occupancy and inflation radii efficiently

Performance Summary Chart

Speedup Factor (vs Old Implementation)
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

Dev Machine (1000×1000):
Old:     ████ 1.0×
New-Off: █████████████ 3.5×
New-On:  ██████████████████████████████████████ 9.6×

Dev Machine (3333×3333):
Old:     ████ 1.0×
New-Off: ███████████ 2.7×
New-On:  ███████████████████████████████████████████████████████████ 15.3×

Robot (1000×1000):
Old:     ████ 1.0×
New-Off: ██████████ 2.6×
New-On:  ████████████████████████████████████████████████ 12.2×

Robot (3333×3333):
Old:     ████ 1.0×
New-Off: ████████ 2.0×
New-On:  ███████████████████████████████████████████████ 11.9×

Codecov Report

❌ Patch coverage is 87.65823% with 39 lines in your changes missing coverage. Please review.

... and 47 files with indirect coverage changes

Yes, there is a problem due to a security PR. The security reports on github doesn't run CI (!!!!!!!!!) so I will be rejecting any patch made that way and requiring them to be public to run CI and properly reviewed. I've asked the contributor to make that fix but if I don't hear back in a couple of days I will do it myself. Ignore the message validation test failures for now, it is not your fault.

However, I wonder if we should make this the PotentialFieldInterfaceLayer and be more generic than specifically inflation.

Not a bad idea but we can also do the renaming once we have other layers that don't fit the description anymore

OK - check DCO and otherwise LGTM after whatever optimization playgrounding / Dexory testing is required to make you comfortable with me merging this + the docs PR

check DCO

I have auto-signoff in vscode, it just doens't work when I revert 😭

after whatever optimization playgrounding / Dexory testing

Will come back in a couple of weeks! If you want to shoutout to potential testers that also need a faster inflation layer, go ahead

btw did you play around with it or the benchmarks yourself?

I did not - I trust you not to make up numbers 😉

LMK when you're ready for merge + link me back to your docs PR to merge at the same time :-)

I tested (rather quickly) implementing some SIMD optimization with the help of Claude but it failed miserably (unreadable and not much performance improvement). If you have a certain direction in mind you can share. Otherwise, I'm ready to merge. I just fixed a merge conflict in ros-navigation/docs.nav2.org#861

cc @doisyg

Codecov didn't upload, I want to see those before merging. I retriggered

@tonynajjar please update one of the nav2 system test configs / launch files to use the legacy inflation layer so it continues to be exercised. This removes all testing of it so its now completely untested 🙃

@SteveMacenski re-added the legacy inflation tests. By the way, I also fixed a build error introduced by the rebase that was not detected by CI because the flag _OPENMP is not enabled. Would be nice to enable CI to build packages with all the possible cmake flag variations.

not detected by CI because the flag _OPENMP is not enabled

Want to add that https://github.com/ros-navigation/navigation2/blob/main/.circleci/config.yml#L218-L221 or https://github.com/ros-navigation/navigation2/blob/main/.circleci/defaults.yaml?

Can we also have one of the system tests run the legacy version too?

Can we also have one of the system tests run the legacy version too?

Ah yeah

Want to add that https://github.com/ros-navigation/navigation2/blob/main/.circleci/config.yml#L218-L221 or https://github.com/ros-navigation/navigation2/blob/main/.circleci/defaults.yaml?

You mean have it build with OPENMP on by default? In this case there is barely any uncovered code for OPENMP disabled so we could go for it but in general I think we should be building with all possible values of flags.
For now I'm fine with turning OPENMP on.

Now we get these warnings because ENABLE_OPENMP is set for all packages

Starting >>> dwb_plugins
--- stderr: dwb_plugins                                                    
CMake Warning:
  Manually-specified variables were not used by the project:

    ENABLE_OPENMP

Probably cleanest to use .meta files
https://colcon.readthedocs.io/en/released/user/configuration.html#package-specific-configuration
But since that infrastructure is not setup, I would ticket that for another time

Regarding the system test using legacy inflation layer, it works -> test 19 doesn't show the log OpenMP: 4 cores available, using 2 threads (auto) which means it's using the legacy inflation layer

@SteveMacenski again ready from my side

@tonynajjar disable openmp then for now; its not worth the trouble. We can deal with it later :-)

Then I can merge once the docs PR is un-conflicted 😉

@tonynajjar disable openmp then for now; its not worth the trouble. We can deal with it later :-)

done

Then I can merge once the docs PR is un-conflicted 😉

What do you mean, it's already merged: ros-navigation/docs.nav2.org#861