Skip to content

Conversation

@rhettinger
Copy link
Contributor

@rhettingerrhettinger commented Jan 8, 2023

On the Apple M1 Max, this change makes no difference. I get 303/304 nsec per loop before and after the edit.

Would anyone care to run this on their builds and report back the results?

% ./python.exe -m timeit -r21 -s 'n=100' -s 'from random import expovariate as r' -s 'v1=[r(1000) + r(0.125) for i in range(n)]' 'sum(v1)' 1000000 loops, best of 21: 304 nsec per loop 

@hauntsaninja
Copy link
Contributor

I see no difference either, on Linux with an AMD Zen 2 chip

@eendebakpt
Copy link
Contributor

Both with and without optimizations I see no difference. System: Linux, gcc version 9.4.0 (Ubuntu 9.4.0-1ubuntu1~20.04.1), Intel(R) Core(TM) i7-4710MQ CPU @ 2.50GHz

@rhettinger
Copy link
ContributorAuthor

Thank you both. It would be nice to hear from a Windows person as well.

@eendebakpt
Copy link
Contributor

Thank you both. It would be nice to hear from a Windows person as well.

On Windows (default PCbuild/build.bat, no PGO) the timings vary a lot on my system (Intel(R) Core(TM) i7-4710MQ CPU @ 2.50GHz, Windows 10, VS 2019). For this PR, measurements within 5 minutes:

image

I can confirm that the minimum time for the test is roughly the same for main and this PR.

@rhettinger
Copy link
ContributorAuthor

Thank you. I appreciate it.

@mdickinson Given that 2Sum and Fast2Sum have the same performance in the context of builtin.sum(), do we have a non-performance reason to choose one over the other? Or should I leave the sum() code as-is?

@rhettingerrhettinger changed the title Timing experiment: For builtin_sum, try replacing Fast2Sum with 2SumGH-100425: Timing experiment: For builtin_sum, try replacing Fast2Sum with 2SumJan 12, 2023
@mdickinson
Copy link
Member

@rhettinger Leaving as-is sounds good to me. The two should be functionally identical, so performance is just about the only thing that would justify choosing one over the other.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@rhettinger@hauntsaninja@eendebakpt@mdickinson@bedevere-bot