Reduce the number of imports for argparse

@rhettinger

BPO	30152
Nosy	@rhettinger, @terryjreedy, @pitrou, @vstinner, @tiran, @methane, @berkerpeksag, @serhiy-storchaka, @wm75, @yan12125
PRs	bpo-30152: Reduce the number of imports for argparse. #1269

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2017-09-25.21:57:20.691>
created_at = <Date 2017-04-24.07:33:50.118>
labels = ['3.7', 'type-feature', 'library']
title = 'Reduce the number of imports for argparse'
updated_at = <Date 2017-09-25.21:57:20.690>
user = 'https://github.com/serhiy-storchaka'

bugs.python.org fields:

activity = <Date 2017-09-25.21:57:20.690>
actor = 'serhiy.storchaka'
assignee = 'bethard'
closed = True
closed_date = <Date 2017-09-25.21:57:20.691>
closer = 'serhiy.storchaka'
components = ['Library (Lib)']
creation = <Date 2017-04-24.07:33:50.118>
creator = 'serhiy.storchaka'
dependencies = []
files = []
hgrepos = []
issue_num = 30152
keywords = []
message_count = 24.0
messages = ['292200', '292219', '292310', '292318', '292320', '292324', '292326', '292328', '292331', '292458', '292512', '292514', '292524', '292528', '292563', '292962', '302816', '302817', '302822', '302835', '302941', '302944', '302979', '302980']
nosy_count = 12.0
nosy_names = ['rhettinger', 'terry.reedy', 'pitrou', 'bethard', 'vstinner', 'christian.heimes', 'methane', 'berker.peksag', 'paul.j3', 'serhiy.storchaka', 'wolma', 'yan12125']
pr_nums = ['1269']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue30152'
versions = ['Python 3.7']

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.

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.

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.

@rhettinger

@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__.

I'd move this into its own PR.

Done. bpo-30166.

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.

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.

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)

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

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.

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.

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

$ 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 ;-)

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%.

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.

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).

I have removed changes for which it was requested and addressed other comments. Can this PR be merged now?

I reviewed your PR, see my questions.

Would you mind to run a new benchmark again, please?

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.

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 bpo-1755179. 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.

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.

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.

New changeset 8110837 by Serhiy Storchaka in branch 'master':
bpo-30152: Reduce the number of imports for argparse. (bpo-1269)
8110837

Thanks all!

Navigation Menu

Repository navigation

Reduce the number of imports for argparse #74338

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Reduce the number of imports for argparse #74338

Description

Activity

serhiy-storchaka commented on Apr 24, 2017

berkerpeksag commented on Apr 24, 2017

rhettinger commented on Apr 26, 2017

wm75 commented on Apr 26, 2017

serhiy-storchaka commented on Apr 26, 2017

vstinner commented on Apr 26, 2017

serhiy-storchaka commented on Apr 26, 2017

serhiy-storchaka commented on Apr 26, 2017

tiran commented on Apr 26, 2017

pitrou commented on Apr 27, 2017

serhiy-storchaka commented on Apr 28, 2017

serhiy-storchaka commented on Apr 28, 2017

vstinner commented on Apr 28, 2017

serhiy-storchaka commented on Apr 28, 2017

terryjreedy commented on Apr 29, 2017

serhiy-storchaka commented on May 4, 2017

serhiy-storchaka commented on Sep 23, 2017

vstinner commented on Sep 23, 2017

serhiy-storchaka commented on Sep 24, 2017

serhiy-storchaka commented on Sep 24, 2017

methane commented on Sep 25, 2017

serhiy-storchaka commented on Sep 25, 2017

serhiy-storchaka commented on Sep 25, 2017

serhiy-storchaka commented on Sep 25, 2017

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions