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.

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

Missing some obvious ast optimizations #123080

Closed
wrongnull opened this issue Aug 16, 2024 · 3 comments
Closed

Missing some obvious ast optimizations #123080

wrongnull opened this issue Aug 16, 2024 · 3 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage type-feature A feature request or enhancement

Comments

@wrongnull
Copy link
Contributor

wrongnull commented Aug 16, 2024 •
edited by bedevere-app bot
Loading

Feature or enhancement

Proposal:

suppose the following code

1 in [1, 2]

it produces such byte code

  0           0 RESUME                   0

  1           2 LOAD_CONST               0 (1)
              4 LOAD_CONST               1 ((1, 2))
              6 CONTAINS_OP              0
              8 RETURN_VALUE

however, interpreter can easily fold such an expression to a constant in the ast_opt stage. This only applies if both ast nodes are Constant_kind. But there is one obstacle. Types are not yet ready and have incomplete mro at the ast_opt stage. So we can't use functions like PyObject_RichCompare for this. We can compare pointers to PyObject instead, since we know that each object in a "constant expression" is internally a singleton (interned strings, integers from -5 to 255, etc.). Thus, for them, the == operator is semantically equal to the is operator. I'm going to make pull request for this

Has this already been discussed elsewhere?

No response given

Links to previous discussion of this feature:

No response

Linked PRs
@wrongnull wrongnull added the type-feature A feature request or enhancement label Aug 16, 2024
@bedevere-app bedevere-app bot mentioned this issue Aug 16, 2024
Closed
@terryjreedy terryjreedy added the performance Performance or resource usage label Aug 16, 2024
@Eclips4 Eclips4 added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Aug 17, 2024
@serhiy-storchaka
Copy link
Member

The optimizer code is intentionally kept simple. Only necessary optimizations are performed, these that are common in user code. For example, '-'*80, 1/3 or 2**32-1.

This idea was already proposed and rejected earlier, for example in #70909. I think that this issue should be closed for the same reasons.

@wrongnull
Copy link
Contributor Author

The optimizer code is intentionally kept simple. Only necessary optimizations are performed, these that are common in user code. For example, '-'*80, 1/3 or 2**32-1.

This idea was already proposed and rejected earlier, for example in #70909. I think that this issue should be closed for the same reasons.

IMO since there is no strict definition of necessity if interpreter CAN optimize something in a reasonable amount of time it SHOULD do so

@serhiy-storchaka
Copy link
Member

It's not done that way. You should have good reasons to do something, not to not do it. Adding new optimizations increases maintenance burden and has a risk of introducing new bugs.

@serhiy-storchaka serhiy-storchaka closed this as not planned Won't fix, can't repro, duplicate, stale Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage type-feature A feature request or enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@serhiy-storchaka @terryjreedy @wrongnull @Eclips4