| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
@nedbat: Would you mind to test this change, maybe also review it? |
Sorry, something went wrong.
There was a problem hiding this comment.
If you want to change these two (and line 424-425) for consistent style :)
Sorry, something went wrong.
There was a problem hiding this comment.
I chose to not change these on purpose, it would be an unrelated change. Someone can work on a PR to update these once this PR is merged ;-)
Sorry, something went wrong.
|
I ran the Python test suite with -X noopt and I got multiple failures. I fixed all of them. I had to make additional changes:
|
Sorry, something went wrong.
|
Azure Pipelines PR: test_venv failed with a timeout (20 min) in the win32 job, but passed when run again. It's unrelated. |
Sorry, something went wrong.
|
Oh wow. When I ran the test suite using -X noopt, I found more and more issues, and I had to modify more ane more code. I didn't expect that I would have to go up to compile() to add a new parameter. It is need indirectly by distutils which uses py_compile. IMHO distutils must simply ignore noopt and always enable compiler optimizations. I don't see much value in distributing non optimized .pyc files on purpose. The common case is to expect optimized .pyc files. I will not have time to finish this PR and review it carefully before Friday (Python 3.8 feature freeze), so I prefer to tag the PR as WIP. |
Sorry, something went wrong.
|
I agree with @vstinner that worrying about compile() is unnecessary. If you view this as an analysis tool then the vast majority of it is going to be coming from .pyc files only. |
Sorry, something went wrong.
|
It is unfortunate that in the current state of the code, you can’t control the compilation of pyc files in distutils build commands with options: the result depends on the -B/-O options passed to the python command itself. I had changed it in distutils2 but thought it could not be changed in distutils. |
Sorry, something went wrong.
There was a problem hiding this comment.
This is a very large change. Are all of them necessary? Could we encode this option as special optimization level or compiler flag? This might keep more code unchanged.
Sorry, something went wrong.
There was a problem hiding this comment.
optimize and noopt. It looks slightly weird.
Sorry, something went wrong.
There was a problem hiding this comment.
I know but I failed to find better names.
Sorry, something went wrong.
There was a problem hiding this comment.
Should not it be controlled by flags or optimization_level?
Sorry, something went wrong.
There was a problem hiding this comment.
optimization_level=0 still means having optimizations enabled. I cannot change to keep the backward compatibility.
Do you think that it would be better to add a new PyCF_xxx flag to avoid adding this new function?
Sorry, something went wrong.
There was a problem hiding this comment.
Maybe use -1 for noopt?
Sorry, something went wrong.
There was a problem hiding this comment.
-1 has a special meaning for compile functions. I prefer to have a separated field to avoid confusion.
Sorry, something went wrong.
|
@serhiy-storchaka: "This is a very large change. Are all of them necessary? Could we encode this option as special optimization level or compiler flag? This might keep more code unchanged." In GCC, gcc -O0 really means "no optimization". In Python, sadly, for historical reasons, optimization_level=0 still means "optimize but only with a few optimizations". It's not "don't optimize". And we cannot change the meaning of optimization_level=0: it would just break too much code. That's why I chose to add a new option. I dislike optimization_level=-1. It sounds really surprising that an optimization level can be negative. |
Sorry, something went wrong.
Add -X noopt command line option to disable compiler optimizations. distutils ignores noopt: always enable compiler optimizations. * Add sys.flags.noopt flag. * Add 'noopt' keyword-only parameter to builtin compile(), importlib.util.cache_from_source() and py_compile.compile(). * importlib: SourceLoader gets an additional keyword-only '_noopt' parameter. It is used by py_compile.compile(). * importlib uses ``.noopt.pyc`` suffix for .pyc files if sys.flags.noopt is true. * Add PyConfig.optimize parameter. * compiler: rename c_optimize to c_optimization_level, add c_optimize. * Update subprocess._optim_args_from_interpreter_flags(). * Add support.requires_compiler_optimizations() * Update unit tests which rely on compiler optimizations for noopt: add @support.requires_compiler_optimizations. * Add _PyAST_Compile() and _Py_CompileString() private functions: they have an additional 'noopt' parameter. * Set _Py_CONFIG_VERSION to 2 Co-Authored-By: Yury Selivanov <yury@magic.io>
|
I rebased my PR:
Latest commit message: commit fa659c13f8d000f7e8b229e5b701df99f778cdc6
Author: Victor Stinner <vstinner@redhat.com>
Date: Mon May 27 23:58:28 2019 +0200
[bpo-2506](https://bugs.python.org/issue2506): Add -X noopt command line option
Add -X noopt command line option to disable compiler optimizations.
distutils ignores noopt: always enable compiler optimizations.
* Add sys.flags.noopt flag.
* Add 'noopt' keyword-only parameter to
builtin compile(), importlib.util.cache_from_source() and
py_compile.compile().
* importlib: SourceLoader gets an additional keyword-only '_noopt'
parameter. It is used by py_compile.compile().
* importlib uses ``.noopt.pyc`` suffix for .pyc files
if sys.flags.noopt is true.
* Add PyConfig.optimize parameter.
* compiler: rename c_optimize to c_optimization_level,
add c_optimize.
* Update subprocess._optim_args_from_interpreter_flags().
* Add support.requires_compiler_optimizations()
* Update unit tests which rely on compiler optimizations for noopt:
add @support.requires_compiler_optimizations.
* Add _PyAST_Compile() and _Py_CompileString() private functions:
they have an additional 'noopt' parameter.
* Set _Py_CONFIG_VERSION to 2
Co-Authored-By: Yury Selivanov <yury@magic.io>
|
Sorry, something went wrong.
|
Open questions:
This PR is quite large, but I'm not sure how to split it into smaller changes. I'm not sure that it can be splitted into smaller atomic changes, since all changes are related. |
Sorry, something went wrong.
I understand why you prefered that option, but I find it very confusing. I agree with Serhiy in that it would be ideal if we can use compiler.c_optimize to signal the no-opt state. |
Sorry, something went wrong.
So I tested and ... it didn't work at all :-( The PEP 587 had a flaw which prevented that! I just fixed the implementation to add a "struct_size" field to PyConfig. So it becomes possible to add new fields to PyConfig in Python 3.9, and be ABI compatible with Python 3.8. The fix: https://bugs.python.org/issue38304 |
Sorry, something went wrong.
New update: the whole idea of a stable ABI for PyConfig has been abandoned. We can freely add new fields in Python 3.9 without having to care about backward compatibility. |
Sorry, something went wrong.
|
I'm no longer interesting to push this feature. The implementation is quite intrusive and @brettcannon didn't like that I passed the "no optimization" flag to so many places. I don't need this feature myself, so I'm not really motivated to fight each individual issue. If someone wants to complete this change, feel free to copy/fork this PR. You can start from https://github.com/python/cpython/pull/13600.patch for example. By the way, this PR was created from an old patch by Yury Selivanov. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Add -X noopt command line option to disable compiler optimizations.
if sys.flags.noopt is true.
add c_optimize.
Co-Authored-By: Yury Selivanov yury@magic.io
https://bugs.python.org/issue2506