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

Rjf/unit create by ref #151

Merged
merged 2 commits into from
Mar 15, 2024
Merged

Rjf/unit create by ref #151

merged 2 commits into from
Mar 15, 2024

Conversation

robfitzgerald
Copy link
Collaborator

this PR makes it so any units functions take references to their arguments instead of pass-by-value semantics, which should reduce a bunch of unnecessary allocations in our core floating point operations.

still, looking at the bottom of the call stack at the code in routee-compass/src/model/unit/builders.rs, i'm not convinced that we have made these operations as fast as they can be. some additional work could be done to ensure we're not allocating temp objects and perhaps bypassing a conversion to the base units too.

@robfitzgerald robfitzgerald requested a review from nreinicke March 15, 2024 19:06
Copy link
Collaborator

@nreinicke nreinicke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me, did you notice any performance improvements after implementing this?

@robfitzgerald
Copy link
Collaborator Author

Didn't run a test to see, I kinda just whipped this up while on a call with tech support for the VPN problems I've been having, and assumed it would help things. 🤷

@robfitzgerald robfitzgerald merged commit 6b3b653 into main Mar 15, 2024
5 checks passed
@robfitzgerald robfitzgerald deleted the rjf/unit-create-by-ref branch March 15, 2024 20:21
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