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

BatInt.Safe_int.mul performance improvements #808

Merged
merged 2 commits into from
Dec 2, 2017

Conversation

murmour
Copy link
Contributor

@murmour murmour commented Nov 19, 2017

This patch makes the fast-path of BatInt.Safe_int.mul less expensive, by getting rid of the absolute value computation (which incurs branching). The downside is that the fast-path supports only non-negative integers from now on.

I am responsible for making the fast-path support negative numbers (#590). It made sense for a particular application of mine, but further experience showed that multiplication of positive integers is much more common in practice, in a wide variety of situations.

The fast-path was made less expensive, by getting rid of absolute value
computation (which incurs branching). The downside is that the fast-path
supports only non-negative integers from now on.
@gasche
Copy link
Member

gasche commented Nov 30, 2017

I looked at this and, while I don't understand the code very well as you do, I believe that the change is correct. Could you add a ChangeLog entry?

@murmour
Copy link
Contributor Author

murmour commented Dec 2, 2017

Done.

@gasche gasche merged commit 4478906 into ocaml-batteries-team:master Dec 2, 2017
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