georust / geographiclib-rs

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

improve performance #32

Open stonylohr opened 3 years ago

stonylohr commented 3 years ago

Some avenues to consider, in roughly chronological order of observation, include:

  1. Issues #1, #2, #3, #4, #5, #6, #7, #8, #9, #10, and #12 for geomath.
  2. Modify geomath::norm to pass values by mutable reference and update them in place rather than returning a separate value.
  3. Negate geodesic._gen_inverse clam12 on the same line where it's assigned, and make it non-mut.
  4. Explore use of f64.sin_cos() over f64.sin() followed by f64.cos().
  5. Explore use of mem::swap() and relatives in place of things like {let c=a; a=b; b=c;}
  6. Look for places that assign the same calculated value more than once in a row, like:
    M12 = sig12.cos();
    M21 = sig12.cos(); // instead use M21 = M12;
  7. Explore reducing scratch array allocations, through some combination of (a) more reuse and (b) larger allocations followed by slicing.
  8. Review choice of declared types, to reduce need for casting.
  9. Make more use of operation-assign operators, like |=.
  10. For optional inputs, standardize (at least for each individual case) on either None or NAN as the "no value" option, rather than using a 2-step translation process (e.g. None to NAN to calculated default).

None is likely to make much impact individually, but they might add up to something together.

I'll continue adding items to this list as I explore issues with returned values, until we get around to actually trying to make performance improvements.

valarauca commented 10 months ago

Actually been attacking these problems on my own.

I Currently have the rust implementation at ~7.06% faster than the reference C implementation. I hope to open a PR by sunday. If the project is interested in these changes.

stonylohr commented 10 months ago

Wow! I'll be interested to see your changes, though it may take me a bit. I haven't had much time for this in a while, but I think I can figure out a way to make some time for it.

On Thu, Jan 18, 2024 at 4:55 PM Cody Laeder @.***> wrote:

Actually been attacking these problems on my own.

I Currently have the rust implementation at ~7.06% faster than the reference C implementation. I hope to open a PR by sunday. If the project is interested in these changes.

— Reply to this email directly, view it on GitHub https://github.com/georust/geographiclib-rs/issues/32#issuecomment-1899460590, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPR6RKLS3F2AEVDI65JZXLYPG77PAVCNFSM4WQKTF52U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCOBZHE2DMMBVHEYA . You are receiving this because you authored the thread.Message ID: @.***>