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

SLOW_SUM flag in sum builtin #119192

Closed
sobolevn opened this issue May 19, 2024 · 2 comments
Closed

SLOW_SUM flag in sum builtin #119192

sobolevn opened this issue May 19, 2024 · 2 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@sobolevn
Copy link
Member

sobolevn commented May 19, 2024

Bug report

Here's a very strange condition that we have in our sum definition:

cpython/Python/bltinmodule.c

Lines 2573 to 2577 in 0abf997

#ifndef SLOW_SUM
/* Fast addition by keeping temporary sums in C instead of new Python objects.
Assumes all inputs are the same type. If the assumption fails, default
to the more general routine.
*/

But, here are the strange parts:

  1. SLOW_SUM is never documented
  2. There are no configure options to set it
  3. There are no other mentions of it in CPython
  4. I was only able to find this mention of it on the internet: https://www.reddit.com/r/Python/comments/18vv3aa/what_is_slow_sum_in_the_cpython_source_code/
  5. I don't think that it should exist, because it might change the builtin if this identifier is somehow magically defined. This would be a bug, because this is never documented / promised by us. It would be quite cryptic bug also
  6. It is very old and still not "official", added in 8ce8a78

I propose to remove it and always have a single way how sum behaves.
I will send a PR.

Linked PRs

@sobolevn sobolevn added type-bug An unexpected behavior, bug, or error interpreter-core (Objects, Python, Grammar, and Parser dirs) labels May 19, 2024
@sobolevn sobolevn self-assigned this May 19, 2024
sobolevn added a commit to sobolevn/cpython that referenced this issue May 20, 2024
@rhettinger
Copy link
Contributor

rhettinger commented May 20, 2024

I wish you would leave this in. It helped greatly in testing, in proving that it really is an optimization, and in segregating the more complex optimization path from the core logic.

Also, as you sweep through the code, please keep in mind Chesterson's Fence and not just remove everything that isn't exactly what you would have done.

And no, this should not be a configuration option. There is no need to present end users with build options they won't benefit from. This symbol is for people who are maintaining this code (in this case, me).

@tim-one
Copy link
Member

tim-one commented May 20, 2024

OTOH, does it actually serve a purpose to give it a name? That is, wouldn't #if 1 allow a 1-character edit to turn it off when that's wanted? You have to recompile to change the behavior anyway.

No strong opinion from me regardless. But if you haven't actually #defined it in, say, the last year, you shouldn't have a strong opinion either 😉.

@gvanrossum gvanrossum closed this as not planned Won't fix, can't repro, duplicate, stale May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

4 participants