Skip to content

Optimized Ord::cmp function#147

Closed
mikem8891 wants to merge 0 commit into
rust-num:mainfrom
mikem8891:ord-cmp
Closed

Optimized Ord::cmp function#147
mikem8891 wants to merge 0 commit into
rust-num:mainfrom
mikem8891:ord-cmp

Conversation

@mikem8891

@mikem8891 mikem8891 commented Feb 25, 2026

Copy link
Copy Markdown

This implementation of the Ord::cmp function does several early comparisons to try to determine ordering before resulting to div and mod operations. This allows about half of unsigned Ratio<t> pairs and 3/4 of signed Ratio<T> pairs to be determined using simple comparisons (based on the entire input space, individual results may vary). If simple comparisons don't determine the ordering, then the function enters a tight loop to converge on a solution as quickly as possible.

Note, #121 would be faster, but would still benefit from this pull request if checked_mul returns None.

I confirmed this fixes #140 . This solution is iterative instead of recursive.

This should also fixes #146 . I phoned this in because I think negative denominators should not be adversely affecting performance or complexity compared to normal fractions.

@mikem8891

Copy link
Copy Markdown
Author

For context, here are some benchmark results

new_test_cmp_overflow   time:   [1.8502 µs 1.8865 µs 1.9358 µs]
Found 17 outliers among 100 measurements (17.00%)
  3 (3.00%) high mild
  14 (14.00%) high severe

old_test_cmp_overflow   time:   [2.3391 µs 2.3740 µs 2.4193 µs]
Found 6 outliers among 100 measurements (6.00%)
  5 (5.00%) high mild
  1 (1.00%) high severe

I can make this benchmark available to you if needed.

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.

Ord:cmp does not properly handle Ratio<T> with negative denominators Stackoverflow with PartialEq and PartialOrd

1 participant