10000 bpo-2506: Experiment with adding a "-X noopt" flag by 1st1 · Pull Request #9693 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

bpo-2506: Experiment with adding a "-X noopt" flag #9693

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
wants to merge 2 commits into from
Closed

bpo-2506: Experiment with adding a "-X noopt" flag #9693

wants to merge 2 commits into from

Conversation

1st1
Copy link
Member
@1st1 1st1 commented Oct 4, 2018 •
edited
Loading

I've created this PR to evaluate if we can add the -X noopt (or similar) option. So far I've done the following basic things:

  • Added the actual command line option
  • Modified the compiler to respect it and to disable AST and peephole optimizers
  • Modified the compiler to respect the flag and avoid optimizing the while <const> and if <const> constructs

Partial list of ToDo's

  • Decide whether it's -X noopt or -O0something
  • Fix importlib to generate "noopt" tags (file.noopt.cpython-38.pyc)
  • Go through compile.c and disable micro-optimizations (like avoiding evaluating test cases of loops in while [constant] etc)
  • Write new tests; fix existing tests.
  • Documentation / What's New / NEWS

I don't have time to implement everything myself right now, but it might be a good idea to ask @elprans to work on this one in the next couple of months (@vstinner currently mentors Elvis.)

https://bugs.python.org/issue2506

(See also https://bugs.python.org/issue34888)

@nedbat
Copy link
Member
nedbat commented Oct 4, 2018

@1st1 thanks!

@warsaw
Copy link
Member
warsaw commented Oct 4, 2018

I'm in favor of being able to disable the peephole optimizer, but I wonder if we need to rethink how we do -O and -OO to make it all more consistent. I don't like that we have those two options and -X noopt. Also, this is just a one-off option; what if we want to get more control over other types of optimization in the future? Maybe we should repurpose -O so you can do -O noopt or -Onoopt? The we might in the future say -Onoopt,nojit,slowonpurpose kind of thing?

@1st1
Copy link
Member Author
1st1 commented Oct 4, 2018 •
edited
Loading

Test file

a = 1 + 2
while 3:
    print(a)
b = 4

With -X noopt

$ python -X noopt -m dis < test.py
  1           0 LOAD_CONST               0 (1)
              2 LOAD_CONST               1 (2)
              4 BINARY_ADD
              6 STORE_NAME               0 (a)

  2     >>    8 LOAD_CONST               2 (3)
             10 POP_JUMP_IF_FALSE       22

  3          12 LOAD_NAME                1 (print)
             14 LOAD_NAME                0 (a)
             16 CALL_FUNCTION            1
             18 POP_TOP
             20 JUMP_ABSOLUTE            8

  4     >>   22 LOAD_CONST               3 (4)
             24 STORE_NAME               2 (b)
             26 LOAD_CONST               4 (None)
             28 RETURN_VALUE

Without -X noopt (current default Python 3.8 behaviour)

(note that line 2 was optimized away)

$ python -m dis < test.py
  1           0 LOAD_CONST               0 (3)
              2 STORE_NAME               0 (a)

  3     >>    4 LOAD_NAME                1 (print)
              6 LOAD_NAME                0 (a)
              8 CALL_FUNCTION            1
             10 POP_TOP
             12 JUMP_ABSOLUTE            4

  4          14 LOAD_CONST               1 (4)
             16 STORE_NAME               2 (b)
             18 LOAD_CONST               2 (None)
             20 RETURN_VALUE

@1st1
Copy link
Member Author
1st1 commented Oct 4, 2018 •
edited
Loading

@warsaw I'd be OK with -O noopt. FWIW I'm also not a big fan of cramming all possible runtime flags into the new -X option.

@elprans
Copy link
Contributor
elprans commented Oct 4, 2018 •
edited
Loading

The we might in the future say -Onoopt,nojit,slowonpurpose kind of thing?

We already have three switchable compilation behaviors: peephole, NDEBUG (assert and __debug__ removal) and the __doc__ stripping, so, roughly:

-O[no-]peephole, -O[no-]strip-debug, and -O[no-]strip-doc.

Arguably, it might be useful to be able to enable docstring stripping without stripping assert as well, so granularity is good.

What's not clear at all is how this would interact with the bytecode cache.

@1st1
Copy link
Member Author
1st1 commented Oct 4, 2018 •
edited
Loading

We already have three switchable compilation behaviors: peephole, NDEBUG (assert and debug removal) and the doc stripping, so, roughly:

We also have an AST optimizer (and Inada-san is working on making it possible to have that optimizer in Python). To simplify things we can make -O[no-]peephole disable the AST optimizer as well.

Arguably, it might be useful to be able to enable docstring stripping without stripping assert as well, so granularity is good.

I agree. Some packages [ab]use docstrings for meta programming and it's not possible to use those packages with -O to disable asserts.

What's not clear at all is how this would interact with the bytecode cache.

One option is to assign a bit flag to every -O variant. Then we can have the resulting integer (as hex?) as a suffix to pyc file: python_module.opt-{OPTHEX}.cpython-38.pyc

@elprans
Copy link
Contributor
elprans commented Oct 4, 2018

One option is to assign a bit flag to every -O variant.

Once the number of flags grows, this would quickly become unmaintainable, as system-installed packages will have to generate a .pyc for every flag combination. A solution for this is to allow writing the bytecode cache to a user-writable directory, which, together with PycInvalidationMode.CHECKED_HASH, might actually be doable now.

@1st1
Copy link
Member Author
1st1 commented Oct 4, 2018

Once the number of flags grows, this would quickly become unmaintainable, as system-installed packages will have to generate a .pyc for every flag combination.

Ah, got it.

@serhiy-storchaka
Copy link
Member

The main problem is interaction with the bytecode cache. See previous discussion. This option should either blindly set sys.dont_write_bytecode to True or set sys.flags.optimize to -1 (or both).

There are other optimizations in bytecode generation. No bytecode is generated for else: and pass lines. We could add NOPs with specific line numbers for tracing these lines.

@nedbat
Copy link
Member
nedbat commented Oct 4, 2018

I appreciate all of the activity! For my purposes, emitting lines for "else:" and "pass" isn't necessary, but I understand the philosophy of doing it.

@brettcannon
Copy link
Member

@elprans E.g. "-O[no-]peephole, -O[no-]strip-debug, and -O[no-]strip-doc" idea, there should be an open issue for this because @ambv has asked for the ability to turn off everything but assert removals. @vstinner and I talked about this when he was doing all of his AST manipulation work for FAT Python a while back.

And with .pyc files now storing their optimization levels this can be made to not impact .pyc files in terms of wiping out other .pyc files. And there is support for writing .pyc files to parallel directory location that got added after PyCon US 2018 thanks to Carl Meyer.

vstinner
vstinner requested changes Oct 10, 2018
View reviewed changes
Copy link
Member
@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The .pyc filename should be different. I suggest ".noopt.pyc" suffix.

At least cache_from_source() and source_from_cache() of importlib._bootstrap_external have to be modified.

It's a little bit surprising to have sys.flags.optimize and sys.flags.noopt_mode. It seems redundant and a source of inconsistency, no? Would it be possible to modify sys.flags.optimize and _PyCoreConfig.optimization_level instead? For example, change the default optimization to level to 1, -X noopt would use 0.

Problem: doctest is hardcoded to skip tests if the optimization level is greater or equal to 2. doctest can easily be fixed, but maybe other 3rd party modules are hardcoded to rely on an exact optimization level.

@bedevere-bot
Copy link

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

pfalcon added a commit to pfalcon/cpython that referenced this pull request Jan 8, 2019
@pfalcon
Python/compile.c: Don't call peephole optimizer on freshly compiled b…
88344ac 
…ytecode.

The patch (well, Py_INCREF) is based on
python#9693 . Unlike that patch, this just
forcibly skips optimizer, no command-line params, etc.

The purpose if producing reference bytecode output for initial stages of
https://github.com/pfalcon/python-compiler upgrade to produce CPython3.5
compatible bytecode (later it's expected to implement peephole optimizer
in Python, to use unmodified CPython code).
@nedbat
Copy link
Member
nedbat commented Feb 7, 2019

@1st1 What are your thoughts about pushing this forward?

@nedbat
Copy link
Member
nedbat commented Apr 23, 2019

@1st1 I would love to see this progress. Is there something I can do to help?

@vstinner
Copy link
Member

It seems like Yury @1st1 was too busy to finish this PR, so I created a new PR to finish it: PR #13600.

@terryjreedy
Copy link
Member

Victor's patch has been closed and followed by Pablo Salgado's #22027.

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

@vstinner vstinner vstinner requested changes

@methane methane Awaiting requested review from methane

@serhiy-storchaka serhiy-storchaka Awaiting requested review from serhiy-storchaka

Assignees
No one assigned
Labels
awaiting changes DO-NOT-MERGE
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@1st1 @nedbat @warsaw @elprans @serhiy-storchaka @brettcannon @bedevere-bot @vstinner @terryjreedy @the-knights-who-say-ni
0