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

bpo-42454: Optimize constant slice creation #23496

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

isidentical
Copy link
Member

@isidentical isidentical commented Nov 24, 2020

Python/ast_opt.c Show resolved Hide resolved
Doc/whatsnew/3.10.rst Outdated Show resolved Hide resolved
@markshannon
Copy link
Member

@markshannon markshannon commented Nov 25, 2020

Why do this in the AST optimizer, rather than the CFG optimizer?
Can this convert tuples containing slices, as used in numpy, into constants? . E.g. [:, 1]

@isidentical
Copy link
Sponsor Member Author

@isidentical isidentical commented Nov 25, 2020

Why do this in the AST optimizer, rather than the CFG optimizer?

How is this different than any kind of constant folding that we do in the AST optimizer? The only reason to move that I can think of is, having CFG to cover more cases than the AST optimizer (like the stuff compiler generates independently from the AST node) but I do not think it is a valid argument for slices (I can't think of any code path that the AST optimizer would miss).

@markshannon
Copy link
Member

@markshannon markshannon commented Nov 25, 2020

That wasn't a "please justify yourself" question, just an "I'm curious" question 🙂
I'm wondering whether we should remove constant folding from the CFG optimizer, if the AST optimizer does it anyway.

It seems that neither optimizer turns (0 if True else 1, None) into a constant (yet).

@markshannon
Copy link
Member

@markshannon markshannon commented Nov 25, 2020

A quick bit of experimentation shows that the CFG optimizer folds default values and annotations, which aren't handled by the AST optimizer.

Copy link
Member

@markshannon markshannon left a comment

LGTM

@isidentical
Copy link
Sponsor Member Author

@isidentical isidentical commented Nov 25, 2020

A quick bit of experimentation shows that the CFG optimizer folds default values and annotations, which aren't handled by the AST optimizer.

How can the CFG optimizer can do extra work on annotations? (Since PEP 563 is enabled by default, they are all strings)? For the default values, I believe this code path should optimize the defaults away, but if there is something wrong with it let's open an issue and discuss.

cpython/Python/ast_opt.c

Lines 620 to 621 in b9127dd

CALL_SEQ(astfold_expr, expr, node_->kw_defaults);
CALL_SEQ(astfold_expr, expr, node_->defaults);

@markshannon
Copy link
Member

@markshannon markshannon commented Nov 25, 2020

Try it for yourself. Comment out fold_tuple_on_constants in compile.c

@isidentical
Copy link
Sponsor Member Author

@isidentical isidentical commented Nov 25, 2020

Try it for yourself. Comment out fold_tuple_on_constants in compile.c

It works fine for defaults?

 $ ./python -m dis
def foo(a = (1,2,3)): pass
  1           0 LOAD_CONST               0 ((1, 2, 3))
              2 BUILD_TUPLE              1
              4 LOAD_CONST               1 (<code object foo at 0x7fde5581e520, file "<stdin>", line 1>)
              6 LOAD_CONST               2 ('foo')
              8 MAKE_FUNCTION            1 (defaults)
             10 STORE_NAME               0 (foo)
             12 LOAD_CONST               3 (None)
             14 RETURN_VALUE

Disassembly of <code object foo at 0x7fde5581e520, file "<stdin>", line 1>:
  1           0 LOAD_CONST               0 (None)
              2 RETURN_VALUE

@markshannon
Copy link
Member

@markshannon markshannon commented Dec 1, 2020

Try running the test suite, and look at the failures. I think it only happens in nested functions with defaults. Not sure why.

@isidentical
Copy link
Sponsor Member Author

@isidentical isidentical commented Dec 1, 2020

Ah, I see. It is not related to the constant folding that I was speaking of (like folding a single default's value a = (1, 2, 3)) but rather the tuple of all default values a=1, b=2 => (1, 2). AFAIK this and some other cases where the compiler generates tuples (via BUILD_TUPLE) but the cases aren't exactly optimizable on the AST is the reason that we do this after the CFG is emitted (BUILD_TUPLE search on Python/compile.c highlights some of these, especially the ones that represents the signatures of function objects).

@github-actions
Copy link

@github-actions github-actions bot commented Jan 1, 2021

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale label Jan 1, 2021
@tiran tiran removed their request for review Apr 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting merge CLA signed stale
Projects
None yet
5 participants