Skip to content

Conversation

@CrazyRoka
Copy link
Contributor

@CrazyRoka CrazyRoka commented Jan 4, 2026

This PR optimizes the "slow path" of seq by replacing the allocation-heavy addition loop with in-place mutation.

Currently, the print_seq loop performs the following operation:

value = value + increment.clone();

It explicitly clones the increment (a potential BigInt allocation) on every iteration.

I have implemented std::ops::AddAssign<&ExtendedBigDecimal> for ExtendedBigDecimal and updated seq to use:

value += &increment;

I reviewed the upstream bigdecimal crate and noticed that addassign_bigdecimal_ref currently contains a TODO and performs a to_owned() internally. Even with that upstream limitation, this PR provides immediate benefits.

@CrazyRoka CrazyRoka force-pushed the seq-optimize-slow-path branch from 19d093e to 6ee9418 Compare January 4, 2026 11:27
@CrazyRoka CrazyRoka closed this Jan 4, 2026
@CrazyRoka CrazyRoka deleted the seq-optimize-slow-path branch January 4, 2026 12:01
@sylvestre
Copy link
Contributor

What happened? :)

@CrazyRoka
Copy link
Contributor Author

I expected CodSpeed to report performance improvement, but that didn't happen.

I think existing benchmarks are not correctly exercising the slow path:

/// Benchmark large integer
#[divan::bench]
fn seq_large_integers(bencher: Bencher) {
bencher.bench(|| {
black_box(run_util_function(uumain, &["4e10003", "4e10003"]));
});
}

Apart from that, improvement might be minimal, because bigdecimal is cloning as well.

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