10000 Speedup math.log by removing AC stuff · Issue #102839 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

Speedup math.log by removing AC stuff #102839

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.

Sign up for GitHub

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

Jump to bottom
Closed
skirpichev opened this issue Mar 20, 2023 · 1 comment
Closed

Speedup math.log by removing AC stuff #102839

skirpichev opened this issue Mar 20, 2023 · 1 comment
Labels
performance Performance or resource usage type-feature A feature request or enhancement

Comments

@skirpichev
Copy link
Member
skirpichev commented Mar 20, 2023 •
edited by bedevere-bot
Loading

In #64385 this functions was converted to the Argument Clinic. Unfortunately, the function signature was "corrupted", probably to prevent showing "magic number" 2.718281828459045 in the function docstring, and this has a noticeable speed impact (~2x regression). In the current main:

$ ./python -m timeit -s 'from math import log' 'log(1.1)'
1000000 loops, best of 5: 341 nsec per loop
$ ./python -m timeit -s 'from math import log' 'log(1.1, 3.2)'
500000 loops, best of 5: 576 nsec per loop

while without the AC stuff (but with METH_FASTCALL):

$ ./python -m timeit -s 'from math import log' 'log(1.1)'
2000000 loops, best of 5: 171 nsec per loop
$ ./python -m timeit -s 'from math import log' 'log(1.1, 3.2)'
1000000 loops, best of 5: 346 nsec per loop

(A different "corruption" was used in the cmath.log, without noticeable speed regression. Unfortunately, it can't be adapted for the math module.)

Given that with the current "state of art" for the AC and the inspect module it's impossible to represent the math.log() correctly (see #89381) - I think it does make sense to revert the AC patch for this function. BTW, the math module has several other functions, that are not converted to the AC, e.g. due to performance reasons (like hypot(), see #30312).

Linked PRs
@skirpichev skirpichev added the type-feature A feature request or enhancement label Mar 20, 2023
@AlexWaygood AlexWaygood added the performance Performance or resource usage label Mar 20, 2023
skirpichev added a commit to skirpichev/cpython that referenced this issue Mar 21, 2023
@skirpichev
pythongh-102839: remove AC for math.log
bdbc875 
Before:
$  ./python -m timeit -s 'from math import log' 'log(1.1)'
1000000 loops, best of 5: 344 nsec per loop
$ ./python -m timeit -s 'from math import log' 'log(1.1, 3.2)'
500000 loops, best of 5: 601 nsec per loop

After:
$  ./python -m timeit -s 'from math import log' 'log(1.1)'
2000000 loops, best of 5: 171 nsec per loop
$ ./python -m timeit -s 'from math import log' 'log(1.1, 3.2)'
1000000 loops, best of 5: 348 nsec per loop
@bedevere-bot bedevere-bot mentioned this issue Mar 21, 2023
Merged
rhettinger pushed a commit that referenced this issue Mar 21, 2023
@skirpichev
gh-102839: remove AC for math.log (GH-102863)
d1a89ce
@rhettinger
Copy link
Contributor

My numbers:

Before PR:

% ./python.exe -m timeit -r 11 -s 'from math import log, log2' 'log2(3.14)'
10000000 loops, best of 11: 20.8 nsec per loop
% ./python.exe -m timeit -r 11 -s 'from math import log, log2' 'log(3.14)'
5000000 loops, best of 11: 48.1 nsec per loop
% ./python.exe -m timeit -r 11 -s 'from math import log, log2' 'log(3.14, 3)'
5000000 loops, best of 11: 76.2 nsec per loop

After PR:

% ./python.exe -m timeit -r 11 -s 'from math import log, log2' 'log2(3.14)'
10000000 loops, best of 11: 21 nsec per loop
% ./python.exe -m timeit -r 11 -s 'from math import log, log2' 'log(3.14)'
10000000 loops, best of 11: 21.6 nsec per loop
% ./python.exe -m timeit -r 11 -s 'from math import log, log2' 'log(3.14, 3)'
5000000 loops, best of 11: 40.5 nsec per loop

Fidget-Spinner pushed a commit to Fidget-Spinner/cpython that referenced this issue Mar 27, 2023
@skirpichev @Fidget-Spinner
pythongh-102839: remove AC for math.log (pythonGH-102863)
4dc1709
warsaw pushed a commit to warsaw/cpython that referenced this issue Apr 11, 2023
@skirpichev @warsaw
pythongh-102839: remove AC for math.log (pythonGH-102863)
8754566
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
performance Performance or resource usage type-feature A feature request or enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@rhettinger @skirpichev @AlexWaygood
0