Reduce the number of imports for argparse

[serhiy-storchaka]

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']

[serhiy-storchaka]

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.

[berkerpeksag]

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.

[rhettinger]

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.

[wm75]

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

[serhiy-storchaka]

I'd move this into its own PR.

Done. bpo-30166.

[vstinner]

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.

[serhiy-storchaka]

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.

[serhiy-storchaka]

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)

[tiran]

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

[pitrou]

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.

[serhiy-storchaka]

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.