georust / geographiclib-rs

A port of geographiclib in Rust.
MIT License
41 stars 9 forks source link

Streamline constants #56

Closed valarauca closed 9 months ago

valarauca commented 9 months ago

Changes:

This is the first patch in a series of PR aiming at improving performance of geodesic inverse calculation.

valarauca commented 9 months ago

Minor changes to a few functions in geomath, sorry. Got a little distracted while copying & pasting stuff.

valarauca commented 9 months ago

Aims to resolve:

michaelkirk commented 9 months ago

There are some nice perf wins here, thanks!

$ cargo bench --bench="*" --  --baseline=main-2024-01-24 
   Compiling geographiclib-rs v0.2.3 (/Users/mkirk/src/georust/geographiclib-rs)
    Finished bench [optimized] target(s) in 1.03s
     Running benches/geodesic_benchmark.rs (target/release/deps/geodesic_benchmark-656e1aa161ec181c)
direct (c wrapper)/default
                        time:   [23.868 µs 23.894 µs 23.923 µs]
                        change: [-0.1047% +0.2586% +0.8488%] (p = 0.32 > 0.05)
                        No change in performance detected.
Found 6 outliers among 100 measurements (6.00%)
  3 (3.00%) high mild
  3 (3.00%) high severe

direct (rust impl)/default
                        time:   [26.527 µs 26.565 µs 26.609 µs]
                        change: [-11.679% -11.528% -11.381%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
  1 (1.00%) low mild
  3 (3.00%) high mild
  4 (4.00%) high severe

inverse (c wrapper)/default
                        time:   [44.686 µs 44.742 µs 44.804 µs]
                        change: [-2.2765% -0.7738% +0.2501%] (p = 0.30 > 0.05)
                        No change in performance detected.
Found 6 outliers among 100 measurements (6.00%)
  2 (2.00%) high mild
  4 (4.00%) high severe

inverse (rust impl)/default
                        time:   [58.362 µs 58.459 µs 58.567 µs]
                        change: [-21.016% -20.859% -20.699%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  2 (2.00%) high mild
  1 (1.00%) high severe

This PR entails a lot of different changes. Would you be willing to break it up a bit to be easier to review?

valarauca commented 9 months ago

Would you be willing to break it up a bit to be easier to review?

I already did or I hope I did? 😅 my eventual goal was to wind up something like this.

I guess I can try to subdivide this further. I am doing 2 different things in 1 PR (standardizing constants & changing i64 to usize).

Let me see if I can reduce the scope slightly