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

Mkirk/usize #58

Merged
merged 5 commits into from
Jan 25, 2024
Merged

Mkirk/usize #58

merged 5 commits into from
Jan 25, 2024

Conversation

michaelkirk
Copy link
Member

  • I agree to follow the project's code of conduct.
  • [n/a] I added an entry to CHANGES.md if knowledge of this change could be valuable to users.

Supersedes #57 - essentially reverting some of the clippy suggestions.

$ 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.11s
     Running benches/geodesic_benchmark.rs (target/release/deps/geodesic_benchmark-656e1aa161ec181c)
direct (c wrapper)/default
                        time:   [24.055 µs 24.116 µs 24.196 µs]
                        change: [+0.5925% +0.8104% +1.0284%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 8 outliers among 100 measurements (8.00%)
  2 (2.00%) low mild
  4 (4.00%) high mild
  2 (2.00%) high severe

direct (rust impl)/default
                        time:   [29.099 µs 29.129 µs 29.163 µs]
                        change: [-3.0292% -2.8670% -2.7039%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
  5 (5.00%) high mild
  3 (3.00%) high severe

inverse (c wrapper)/default
                        time:   [44.773 µs 44.843 µs 44.924 µs]
                        change: [-2.0456% -0.5606% +0.4443%] (p = 0.49 > 0.05)
                        No change in performance detected.
Found 6 outliers among 100 measurements (6.00%)
  4 (4.00%) high mild
  2 (2.00%) high severe

inverse (rust impl)/default
                        time:   [77.147 µs 77.215 µs 77.290 µs]
                        change: [+4.3024% +4.4502% +4.6044%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 6 outliers among 100 measurements (6.00%)
  2 (2.00%) high mild
  4 (4.00%) high severe

I did some profiling to see if I could account for the losses in the "inverse" case. Seemingly the biggest offender is an increase in the time spent in atan2. We haven't explicitly changed how we call that method, so I'm not sure what to make of it, but I'm willing to call the gains and losses a wash vs main, and merge this based on the readability improvements.

auto-merge was automatically disabled January 25, 2024 19:27

Merge queue setting changed

@michaelkirk michaelkirk merged commit 9f2b3b9 into main Jan 25, 2024
4 checks passed
@michaelkirk michaelkirk mentioned this pull request Jan 25, 2024
2 tasks
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.

2 participants