Skip to content

Commit

Permalink
optimise sub (#280)
Browse files Browse the repository at this point in the history
Replacing 2 subs or 1 add + 1 sub
with
1 sub or 1 add plus 1 sub

<!--- Please provide a general summary of your changes in the title
above -->

## Pull Request type

<!-- Please try to limit your pull request to one type; submit multiple
pull requests if needed. -->

Minor sub optimise, figured this recently...

Please check the type of change your PR introduces:

- [ ] Bugfix
- [ ] Feature
- [ ] Code style update (formatting, renaming)
- [x] Refactoring (no functional changes, no API changes)
- [ ] Build-related changes
- [ ] Documentation content changes
- [ ] Other (please describe):

## What is the current behavior?

<!-- Please describe the current behavior that you are modifying, or
link to a relevant issue. -->

Subtract does unnecessary ops

Issue Number: N/A

## What is the new behavior?

Comparison operator is subtraction behind the scenes, when we compare a
and b, we don't need to do the sub again.
`if` is replaced with a `match` that just returns the diff right away
when there's no overflow (it'll be much better for non worst case
branch).

```diff
- test alexandria_math::tests::mod_arithmetics_test::sub_mod_1_test ... ok (gas usage est.: 678590)
+ test alexandria_math::tests::mod_arithmetics_test::sub_mod_1_test ... ok (gas usage est.: 615870)
- test alexandria_math::tests::mod_arithmetics_test::sub_mod_2_test ... ok (gas usage est.: 678590)
+ test alexandria_math::tests::mod_arithmetics_test::sub_mod_2_test ... ok (gas usage est.: 615870)
- test alexandria_math::tests::mod_arithmetics_test::sub_mod_test ... ok (gas usage est.: 703270)
+ test alexandria_math::tests::mod_arithmetics_test::sub_mod_test ... ok (gas usage est.: 640550)
```

## Does this introduce a breaking change?

- [ ] Yes
- [*] No

<!-- If this does introduce a breaking change, please describe the
impact and migration path for existing applications below. -->

## Other information

<!-- Any other information that is important to this PR, such as
screenshots of how the component looks before and after the change. -->
  • Loading branch information
shramee authored Feb 26, 2024
1 parent 9e3565d commit c6e777e
Showing 1 changed file with 7 additions and 3 deletions.
10 changes: 7 additions & 3 deletions src/math/src/mod_arithmetics.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,14 @@ fn sub_mod(mut a: u256, mut b: u256, modulo: u256) -> u256 {
// reduce values
a = a % modulo;
b = b % modulo;
if (a >= b) {
return a - b;
let (diff, overflow) = integer::u256_overflow_sub(a, b);
if overflow {
// Overflow back with add modulo
let (diff, _) = integer::u256_overflowing_add(diff, modulo);
diff
} else {
diff
}
a + add_inverse_mod(b, modulo)
}

/// Function that performs modular multiplication.
Expand Down

0 comments on commit c6e777e

Please sign in to comment.