classification
Title: Reduce the number of imports for argparse
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: bethard Nosy List: berker.peksag, bethard, christian.heimes, inada.naoki, paul.j3, pitrou, rhettinger, serhiy.storchaka, terry.reedy, vstinner, wolma, yan12125
Priority: normal Keywords:

Created on 2017-04-24 07:33 by serhiy.storchaka, last changed 2017-09-25 21:57 by serhiy.storchaka. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 1269 merged serhiy.storchaka, 2017-04-24 07:35
Messages (24)
msg292200 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-04-24 07:33
Since argparse becomes more used as standard way of parsing command-line arguments, the number of imports involved when import argparse becomes more important. Proposed patch reduces that number by 10 modules.

Unpatched:

$ ./python -c 'import sys; s = set(sys.modules); import argparse; print(len(s), len(sys.modules), len(set(sys.modules) - s)); print(sorted(set(sys.modules) - s))'
35 65 30
['_collections', '_functools', '_heapq', '_locale', '_operator', '_sre', '_struct', 'argparse', 'collections', 'collections.abc', 'copy', 'copyreg', 'enum', 'functools', 'gettext', 'heapq', 'itertools', 'keyword', 'locale', 'operator', 're', 'reprlib', 'sre_compile', 'sre_constants', 'sre_parse', 'struct', 'textwrap', 'types', 'warnings', 'weakref']

$ ./python -S -c 'import sys; s = set(sys.modules); import argparse; print(len(s), len(sys.modules), len(set(sys.modules) - s)); print(sorted(set(sys.modules) - s))'
23 61 38
['_collections', '_collections_abc', '_functools', '_heapq', '_locale', '_operator', '_sre', '_stat', '_struct', 'argparse', 'collections', 'collections.abc', 'copy', 'copyreg', 'enum', 'errno', 'functools', 'genericpath', 'gettext', 'heapq', 'itertools', 'keyword', 'locale', 'operator', 'os', 'os.path', 'posixpath', 're', 'reprlib', 'sre_compile', 'sre_constants', 'sre_parse', 'stat', 'struct', 'textwrap', 'types', 'warnings', 'weakref']

Patched:

$ ./python -c 'import sys; s = set(sys.modules); import argparse; print(len(s), len(sys.modules), len(set(sys.modules) - s)); print(sorted(set(sys.modules) - s))'
35 55 20
['_collections', '_functools', '_locale', '_operator', '_sre', 'argparse', 'collections', 'copyreg', 'enum', 'functools', 'itertools', 'keyword', 'operator', 're', 'reprlib', 'sre_compile', 'sre_constants', 'sre_parse', 'types', 'weakref']

$ ./python -S -c 'import sys; s = set(sys.modules); import argparse; print(len(s), len(sys.modules), len(set(sys.modules) - s)); print(sorted(set(sys.modules) - s))'
23 51 28
['_collections', '_collections_abc', '_functools', '_locale', '_operator', '_sre', '_stat', 'argparse', 'collections', 'copyreg', 'enum', 'errno', 'functools', 'genericpath', 'itertools', 'keyword', 'operator', 'os', 'os.path', 'posixpath', 're', 'reprlib', 'sre_compile', 'sre_constants', 'sre_parse', 'stat', 'types', 'weakref']

The patch defers importing rarely used modules. For example textwrap and gettext are used only for output a help and error messages.

The patch also makes argparse itself be imported only when the module is used as a script, not just imported. The patch also replaces importing collections.abc with _collections_abc in some other basic modules (like pathlib), this could allow to avoid importing the collections package if it is not used.

Unavoided imports:

* functools is used in re for decorating _compile_repl with lru_cache.
* collections is used in functools for making CacheInfo a named tuple.
* enum is used in re for creating RegexFlag.
* types is used in enum for decorating some properties with DynamicClassAttribute.
msg292219 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2017-04-24 10:58
> The patch also makes argparse itself be imported only when the module
> is used as a script, not just imported.

+1. I'd move this into its own PR.
msg292310 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-04-26 06:58
Please stop rearranging everything in the standard library.  Every day I look at the tracker and there is a new patch from this dev that alters dozens of files.  

Some parts of the patch are harmless but other parts needlessly 
constipates the code for very little benefit.  I would be embarrassed to have to explain why core developers think it is a best practice to move a heapq import inside a Counter method.  That isn't a PEP-8 recommended practice.  Ordinarily, we only recommend deferred imports in extreme cases where the resource might not be available at import time or when the import is *very* expensive.

In general, we've already committed too many sins in the name of optimizing start-up time.  Mostly, these are false improvements because many of the changes just defer imports that ultimately end-up being needed anyway.  As we make useful tools, we're going to want to use them in the standard library to make the code better, but that is going to entail greater inter-dependencies and cross-imports.  Trying to avoid these is a losing game.
msg292318 - (view) Author: Wolfgang Maier (wolma) * Date: 2017-04-26 08:21
@rhettinger: I do not quite understand this harsh reaction. Making argparse more responsive could, in fact, be quite a nice improvement. This is particularly true, I guess, if you want argument completion with a package like https://pypi.python.org/pypi/argcomplete.

You have a point that the patch probably tries to optimize too many things in too many places at once, but that could be amended rather easily.

The idea of delaying imports in argparse itself to when they are actually needed is a really good one and, for this module, it is also very reasonable that other places in the stdlib only import it when they are run as __main__.
msg292320 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-04-26 08:40
> I'd move this into its own PR.

Done. Issue30166.
msg292324 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-04-26 08:48
Raymond Hettinger added the comment:
> Some parts of the patch are harmless but other parts needlessly constipates the code for very little benefit.

In general, Python startup is currently the most severe performance
regression in Python 3 compared to Python 2.7:
https://speed.python.org/comparison/?exe=12%2BL%2Bmaster%2C12%2BL%2B3.5%2C12%2BL%2B3.6%2C12%2BL%2B2.7&ben=649&env=1&hor=true&bas=12%2BL%2B2.7&chart=normal+bars

Python 3.7 is the fastest of Python 3 versions, but Python 3.7 still
more than 2.2x slower than Python 2.7. 2.2x is a large number, it's
not a tiny slowdown.

I agree with Serhiy that argparse became more popular so became more
important for Python startup time in applications.

Maybe Serhiy can try to run a benchmark for get timing (milliseconds)
instead of comparing the number of imports?

Sorry, I didn't have time to review the change itself yet. But I like
efforts to optimize Python startup time in general.
msg292326 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-04-26 09:01
collections is very popular module. It is almost impossible to avoid importing it since so much stdlib modules use namedtuple, OrderedDict, deque, or defaultdict. The reason of moving a heapq import inside a Counter method is that importing heapq adds two entries in sys.modules ('heapq' and '_heapq'), but it is needed only when Counter.most_common() is called with non-default argument. The Counter class is less used than other four popular collections classes, not every use of the Counter class involves using of the most_common() method, and the most_common() method not always is called with argument (actually it is used only twice in the stdlib, in both cases without argument).

I'll remove this change if you say this, but this will decrease the effect of this patch from 10 to 8 modules.
msg292328 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-04-26 09:39
In general I agree with Raymond's principle and I am not very happy doing these changes. But heavy startup is a sore spot of CPython.

For comparison Python 2.7 and optparse:

$ python -c 'import sys; s = set(sys.modules); import argparse; print(len(s), len(sys.modules), len(set(sys.modules) - s))'
(43, 63, 20)
$ python -S -c 'import sys; s = set(sys.modules); import argparse; print(len(s), len(sys.modules), len(set(sys.modules) - s))'
(15, 57, 42)
$ python -c 'import sys; s = set(sys.modules); import optparse; print(len(s), len(sys.modules), len(set(sys.modules) - s))'
(43, 56, 13)
$ python -S -c 'import sys; s = set(sys.modules); import optparse; print(len(s), len(sys.modules), len(set(sys.modules) - s))'
(15, 50, 35)
msg292331 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2017-04-26 10:14
Instead of messing with all modules, we should rather try to improve startup time with lazy imports first. Mercurial added an on-demand importer to speed up CLI performance. Many years ago I worked on PEP 369 for lazy import. It should be much easier to implement with Brett's import system. We'd to come up with a syntax, maybe something like:

with lazy import gettext
with lazy from foo import bar


Or we could re-use async keyword:

async import gettext
async from foo import bar
msg292458 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2017-04-27 16:06
I'd like to vote for a lazy import system that would benefit everyone (many third-party packages are also affected by startup time issues), but I've seen enough handwaving about it along the years that I'm not really hoping any soon.  My own limited attempts at writing one have failed miserably.

In other words, I think Serhiy's proposal is a good concrete improvement over the statu quo.
msg292512 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-04-28 05:43
I just realized that is is not so easy to avoid gettext import. gettext is used in the ArgumentParser constructor for localizing names of default groups and the default help (besides they are used only for formatting help). This adds importing the locale module.
msg292514 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-04-28 06:05
After reverting some changes (import heapq on Raymond's request and import gettext since it is needed for the ArgumentParser constructor) the effect of the patch is reducing the number of imports by 6 and the time by 0.017 sec (> 10%) on my computer.

$ time for i in `seq 100`; do ./python -S -c 'import argparse; argparse.ArgumentParser()'; done

Unpatched:
real    0m13.236s
user    0m12.100s
sys     0m0.808s

Patched:
real    0m11.535s
user    0m10.356s
sys     0m0.756s
msg292524 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-04-28 10:34
> $ time for i in `seq 100`; do ./python -S -c 'import argparse; argparse.ArgumentParser()'; done

Measuring Python startup performance is painful, there is a huge deviation. You may try the new "command" command that I added to perf 1.1:
-----------------------
haypo@selma$ python3 -m perf command --stats -- python3 -S -c pass
.....................
Total duration: 21.0 sec
Start date: 2017-04-28 12:15:57
End date: 2017-04-28 12:16:20
Raw value minimum: 174 ms
Raw value maximum: 229 ms

Number of calibration run: 1
Number of run with values: 20
Total number of run: 21

Number of warmup per run: 1
Number of value per run: 3
Loop iterations per value: 16
Total number of values: 60

Minimum:         10.9 ms
Median +- MAD:   12.3 ms +- 0.5 ms
Mean +- std dev: 12.4 ms +- 0.7 ms
Maximum:         14.3 ms

  0th percentile: 10.9 ms (-12% of the mean) -- minimum
  5th percentile: 11.2 ms (-10% of the mean)
 25th percentile: 11.9 ms (-4% of the mean) -- Q1
 50th percentile: 12.3 ms (-1% of the mean) -- median
 75th percentile: 12.9 ms (+4% of the mean) -- Q3
 95th percentile: 13.7 ms (+10% of the mean)
100th percentile: 14.3 ms (+15% of the mean) -- maximum

Number of outlier (out of 10.4 ms..14.4 ms): 0

command: Mean +- std dev: 12.4 ms +- 0.7 ms
-----------------------

There is a huge difference between the minimum and the maximum.

By the way, I'm interested by feedback on that tool, I'm not sure that it's reliable, it can likely be enhanced somewhere ;-)
msg292528 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-04-28 11:23
> Measuring Python startup performance is painful, there is a huge deviation.

That is why I run Python 100 times and repeat that several times for testing that the result is stable. With perf I got roughly the same result -- the absolute difference is 18-19 ms, or 17%.
msg292563 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-04-29 04:26
I believe importing argparse in the __main__ clause (or a main or test function called therein) is common.  I prefer it there as it is not germain to the code that is usually imported.  I have a similar view on not immediately importing warnings when only needed for deprecation.

I am interested in argparse import time because I have thought about switching IDLE arg processing from getopt to argparse.  But both IDLE and user process startup time are already slow enough.  (For IDLE, the numbers for Serhiy's import counter for 3.6 on Win 10 with idlelib.idle instead of argparse are 38 192 154.

I recently sped up user process startup (performed each time one 'runs' code from the editor) by around 25% (.1 second on my machine, just noticeable) by a few fairly straightforword changes.  For idlelib.run, the import numbers are 40 182 142 in 3.5.3 and 38 138 100 in 3.6.1.  I probably can improve this further, but each change would have to be justified on its own.  So I think it appropriate to start with a subset of possible speedup changes for argparse.
msg292962 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-05-04 05:38
Implemented Wolfgang Maier's idea. The copy module is now imported in argparse only when the default value for 'append' or 'append_const' is not a list (uncommon case).
msg302816 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-09-23 22:36
I have removed changes for which it was requested and addressed other comments. Can this PR be merged now?
msg302817 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-09-23 23:08
I reviewed your PR, see my questions.

Would you mind to run a new benchmark again, please?
msg302822 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-09-24 00:03
On my new laptop 100 runs are sped up from 2.357 sec to 2.094 sec (by 13%). Got rid of importing 6 modules: _struct, collections.abc, copy, struct, textwrap, warnings.

Getting rid of importing errno leads to rewriting of os._execvpe() which looks less trivial. I leave this for separate PR.
msg302835 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-09-24 08:14
Got rid of importing errno as Victor suggested. In gettext its import is deferred. It is only used in one exceptional case. In os errno no longer used.

Initially errno was imported in os inside a function. The import was moved at module level for fixing issue1755179. But now we can get rid of it totally by catching specialized OSError subclasses instead of testing the OSError's errno attribute.

There are other cleaning up changes in the os._execvpe() function. os.path.split() is replaced with os.path.dirname() since only the dirname is used. Saving tracebacks no longer needed because a traceback is a part of an exception in Python 3.
msg302941 - (view) Author: Inada Naoki (inada.naoki) * (Python committer) Date: 2017-09-25 10:18
Oh, the pull request is far larger than I thought!

I doubt that avoiding functools and collections is worth enough, because it is very common.

For example:

* functools is very commonly imported, especially for wraps().

* collections is imported by functools, so it's more commonly imported than functools.

* Old style namespace package (.pth file) imports types module, and types module and types imports functools, collections.  So even `python -c 42` imports them if at least one old-style namespace package is installed.  (e.g. Sphinx)


Instead of avoiding them, I want to make them faster.

* C implementation of ABC will makes collection, weakref and some other modules much faster.
* Recent PEP will allows split functools module into submodules without breaking backward compatibility.
msg302944 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-09-25 10:45
functools.wraps() is used in more complex programs. The purpose of this PR is speeding up small scripts that just start, parse arguments, do its fast work (so fast as outputting a help or a version) and exit. Even collections is not used in a half of my scripts.

types needs functools only for coroutine(). Unlikely it is used in old style namespace packages.
msg302979 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-09-25 21:56
New changeset 81108375d9b2ccd0add1572da745311d4dfac505 by Serhiy Storchaka in branch 'master':
bpo-30152: Reduce the number of imports for argparse. (#1269)
https://github.com/python/cpython/commit/81108375d9b2ccd0add1572da745311d4dfac505
msg302980 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-09-25 21:57
Thanks all!
History
Date User Action Args
2017-09-25 21:57:20serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg302980

stage: patch review -> resolved
2017-09-25 21:56:01serhiy.storchakasetmessages: + msg302979
2017-09-25 10:45:48serhiy.storchakasetmessages: + msg302944
2017-09-25 10:18:07inada.naokisetnosy: + inada.naoki
messages: + msg302941
2017-09-24 08:14:26serhiy.storchakasetmessages: + msg302835
2017-09-24 00:03:18serhiy.storchakasetmessages: + msg302822
2017-09-23 23:08:38vstinnersetmessages: + msg302817
2017-09-23 22:36:55serhiy.storchakasetmessages: + msg302816
2017-05-04 05:38:49serhiy.storchakasetmessages: + msg292962
2017-04-29 04:26:53terry.reedysetnosy: + terry.reedy
messages: + msg292563
2017-04-28 11:23:10serhiy.storchakasetmessages: + msg292528
2017-04-28 10:34:51vstinnersetmessages: + msg292524
2017-04-28 06:05:21serhiy.storchakasetmessages: + msg292514
2017-04-28 05:43:16serhiy.storchakasetmessages: + msg292512
2017-04-27 16:06:20pitrousetnosy: + pitrou
messages: + msg292458
2017-04-26 10:14:26christian.heimessetnosy: + christian.heimes
messages: + msg292331
2017-04-26 09:39:10serhiy.storchakasetmessages: + msg292328
2017-04-26 09:01:19serhiy.storchakasetmessages: + msg292326
2017-04-26 08:48:44vstinnersetmessages: + msg292324
2017-04-26 08:40:09serhiy.storchakasetmessages: + msg292320
2017-04-26 08:21:03wolmasetmessages: + msg292318
2017-04-26 06:58:30rhettingersetassignee: bethard
messages: + msg292310
2017-04-26 06:09:01paul.j3setnosy: + paul.j3
2017-04-24 17:28:06wolmasetnosy: + wolma
2017-04-24 10:58:00berker.peksagsetnosy: + berker.peksag
messages: + msg292219
2017-04-24 08:59:27vstinnersetnosy: + yan12125
2017-04-24 08:59:10vstinnerlinkissue29442 dependencies
2017-04-24 07:35:37serhiy.storchakasetpull_requests: + pull_request1381
2017-04-24 07:33:50serhiy.storchakacreate