◐ Shell
reader mode source ↗
Skip to content

bpo-37884: Optimize Fraction() and statistics.mean()#15329

Closed
serhiy-storchaka wants to merge 5 commits into
python:masterfrom
serhiy-storchaka:math-as_integer_ratio2
Closed

bpo-37884: Optimize Fraction() and statistics.mean()#15329
serhiy-storchaka wants to merge 5 commits into
python:masterfrom
serhiy-storchaka:math-as_integer_ratio2

Conversation

@serhiy-storchaka

@serhiy-storchaka serhiy-storchaka commented Aug 19, 2019

Copy link
Copy Markdown
Member

@jdemeyer

Copy link
Copy Markdown
Contributor

Am I right that this is the same as #15210 but with a private function instead?

@aeros aeros left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

I performed the same tests mentioned in the bpo issue to verify the results and compared between the latest commit to master (master:24fe46081b) to the PR branch (math-as_integer_ratio2:e658d19083).

OS: Arch Linux 5.2.8
CPU: Intel i5-4460

./python -m timeit -s "from fractions import Fraction as F" "F(123)"
200000 loops, best of 5
Master: 1.42 usec
PR: 1.55 usec

./python -m timeit -s "from fractions import Fraction as F" "F(1.23)"
100000 loops, best of 5
Master: 2.92 usec per loop
PR: 2.14 usec

 ./python -m timeit -s "from fractions import Fraction as F; f = F(22, 7)" "F(f)"
100000 loops, best of 5
Master: 2.47 usec
PR: 1.93 usec

./python -m timeit -s "from statistics import mean; a = [1]*1000" "mean(a)"
500 loops, best of 5
Master: 930 usec
PR: 640 usec

./python -m timeit -s "from statistics import mean; a = [1.23]*1000" "mean(a)"
200 loops, best of 5
Master: 1.31 msec
PR: 1.34 msec

./python -m timeit -s "from statistics import mean; from fractions import Fraction as F; a = [F(22, 7)]*1000" "mean(a)"
200 loops, best of 5
Master: 1.31 msec
PR: 1.09 msec

./python -m timeit -s "from statistics import mean; from decimal import Decimal as D; a = [D('1.23')]*1000" "mean(a)"
100 loops, best of 5
Master: 2.08 msec
PR: 2 msec

@aeros

aeros commented Aug 19, 2019

Copy link
Copy Markdown
Contributor

Am I right that this is the same as #15210 but with a private function instead?

It also looks like the Travis doctest and docbuild was failing on the other PR. As far as functionality goes, the changes to the code in mathmodule.c look to be identical, other than the additional underscore in the function name to denote it as private.

@jdemeyer jdemeyer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

Supporting arbitrary objects with an as_integer_ratio method is a non-trivial change and should be documented in a NEWS entry.

@serhiy-storchaka

Copy link
Copy Markdown
Member Author

Am I right that this is the same as #15210 but with a private function instead?

Yes, it is.

Supporting arbitrary objects with an as_integer_ratio method is a non-trivial change and should be documented in a NEWS entry.

Agreed.

@serhiy-storchaka serhiy-storchaka deleted the math-as_integer_ratio2 branch August 24, 2019 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting core review performance Performance or resource usage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants