Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix Inefficient Cloning in From<BigDecimal> for MmNumber #2338

Open
laruh opened this issue Feb 5, 2025 · 0 comments
Open

Fix Inefficient Cloning in From<BigDecimal> for MmNumber #2338

laruh opened this issue Feb 5, 2025 · 0 comments
Assignees
Labels
improvement: resource usage Improvements made on resource usage on runtime or/and compile time priority: Backlog Tasks not yet scheduled or lower priority.

Comments

@laruh
Copy link
Member

laruh commented Feb 5, 2025

The current implementation of From for MmNumber unnecessarily takes ownership of BigDecimal, but then immediately borrows it with from_dec_to_ratio(&n), making the move redundant. Since the function only needs a reference, we should implement From<&BigDecimal> to allow efficient conversion without cloning.
see #2112 (comment) - we clone BigDecimal amount in so many places.

impl From<BigDecimal> for MmNumber {
fn from(n: BigDecimal) -> MmNumber { from_dec_to_ratio(&n).into() }
}

Rust allows implementing both From<BigDecimal> and From<&BigDecimal>, we can support both ownership-based and reference-based conversions.

@laruh laruh added improvement: resource usage Improvements made on resource usage on runtime or/and compile time priority: Backlog Tasks not yet scheduled or lower priority. labels Feb 5, 2025
@borngraced borngraced self-assigned this Feb 19, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
improvement: resource usage Improvements made on resource usage on runtime or/and compile time priority: Backlog Tasks not yet scheduled or lower priority.
Projects
None yet
Development

No branches or pull requests

2 participants