test.support has way too many imports #84456
Comments
|
test_threading.ThreadJoinOnShutdown.test_reinit_tls_after_fork() does crash on AIX: bpo-40068. One of the issue is that logging does crash at fork: bpo-40091. The strange thing is that test_threading doesn't use logging. I'm quite sure that logging is imported through test.support. "import test.support" imports not less than 171... That's quite "heavy". It includes "heavy" modules like concurrent.futures, asyncio or multiprocessing. It's maybe time to slice again test.support until submodules to reduce the number of imports to the bare minimum. For example, "import test.support" imports asyncio, whereas it's only used by very few tests. Maybe we should add "test.support.asyncioutils". $ ./python
Python 3.9.0a5+ (heads/pycore_interp:a1ff2c5cf3, Apr 13 2020, 12:27:13)
>>> import sys
>>> import test
>>> len(after - before)
171
>>> import pprint
>>> pprint.pprint(sorted(after - before))
['__mp_main__',
'_asyncio',
'_bisect',
'_blake2',
'_bz2',
'_collections',
'_compat_pickle',
'_compression',
'_contextvars',
'_datetime',
'_elementtree',
'_functools',
'_hashlib',
'_heapq',
'_lzma',
'_opcode',
'_operator',
'_pickle',
'_posixsubprocess',
'_queue',
'_random',
'_sha3',
'_sha512',
'_socket',
'_sre',
'_ssl',
'_string',
'_struct',
'_sysconfigdata_d_linux_x86_64-linux-gnu',
'_weakrefset',
'argparse',
'array',
'asyncio',
'asyncio.base_events',
'asyncio.base_futures',
'asyncio.base_subprocess',
'asyncio.base_tasks',
'asyncio.constants',
'asyncio.coroutines',
'asyncio.events',
'asyncio.exceptions',
'asyncio.format_helpers',
'asyncio.futures',
'asyncio.locks',
'asyncio.log',
'asyncio.protocols',
'asyncio.queues',
'asyncio.runners',
'asyncio.selector_events',
'asyncio.sslproto',
'asyncio.staggered',
'asyncio.streams',
'asyncio.subprocess',
'asyncio.tasks',
'asyncio.transports',
'asyncio.trsock',
'asyncio.unix_events',
'base64',
'binascii',
'bisect',
'bz2',
'collections',
'collections.abc',
'concurrent',
'concurrent.futures',
'concurrent.futures._base',
'contextlib',
'contextvars',
'copy',
'copyreg',
'datetime',
'difflib',
'dis',
'email',
'email.base64mime',
'email.charset',
'email.encoders',
'email.errors',
'email.header',
'email.quoprimime',
'enum',
'errno',
'faulthandler',
'fnmatch',
'functools',
'gc',
'gettext',
'glob',
'grp',
'gzip',
'hashlib',
'heapq',
'importlib',
'importlib._bootstrap',
'importlib._bootstrap_external',
'importlib.abc',
'importlib.machinery',
'importlib.util',
'inspect',
'itertools',
'keyword',
'linecache',
'locale',
'logging',
'logging.handlers',
'lzma',
'math',
'multiprocessing',
'multiprocessing.context',
'multiprocessing.process',
'multiprocessing.reduction',
'nntplib',
'opcode',
'operator',
'pickle',
'platform',
'pprint',
'pwd',
'pyexpat',
'pyexpat.errors',
'pyexpat.model',
'queue',
'quopri',
'random',
're',
'reprlib',
'resource',
'select',
'selectors',
'shutil',
'signal',
'socket',
'sre_compile',
'sre_constants',
'sre_parse',
'ssl',
'string',
'struct',
'subprocess',
'sysconfig',
'tempfile',
'test.support',
'test.support.testresult',
'threading',
'token',
'tokenize',
'traceback',
'types',
'typing',
'typing.io',
'typing.re',
'unittest',
'unittest.async_case',
'unittest.case',
'unittest.loader',
'unittest.main',
'unittest.result',
'unittest.runner',
'unittest.signals',
'unittest.suite',
'unittest.util',
'urllib',
'urllib.error',
'urllib.response',
'warnings',
'weakref',
'xml',
'xml.etree',
'xml.etree.ElementPath',
'xml.etree.ElementTree',
'zlib'] |
If we split some "heavy" modules to xxxutils, what benefits will we make in fact(more exact module importing behavior)? |
|
+1 for splitting test.support on several submodules! Some imports can also be lazy. |
|
asyncio is now imported in unittest. Removing direct import from test.support does not help. |
Oh, thanks, you are right. Looks like we need check which submodules should be splitted. |
|
logging is also imported in unittest and indirectly in asyncio. |
|
These three PRs combined reduce the number of imported modules from 179 to 105 (not including 24 modules imported by the bare interpreter) and reduce the hot import time from 160 ms to 80 ms. |
Good job, serhiy. I tested your patches in my vm, the number of importing module have been reduced. Could you paste your performance benchmark test? I got a no siginificant change :( when I run: python: ..................................................................................................... 809 ns +- 201 ns Mean +- std dev: [python3.9] 820 ns +- 206 ns -> [python] 809 ns +- 201 ns: 1.01x faster (-1%) |
|
I used -X importtime. Your benchmark measures the performance of look up in sys.modules. |
In master branch: After apply serhiy's patches: |
|
What is the practical impact on the time taken for test runs? How much does the total time for a test run reduce as a result of refactoring test.support to minimise imports? |
|
Reducing the import time is not a goal, it is just a measurable side effect. The goal is to reduce the number of imported modules unneeded for the particular test. Importing every module can have side effects which affects the tested behavior. It would be nice to minimize it. |
|
Vinay Sajip: "What is the practical impact on the time taken for test runs?" My first concern is that our "unit tests" are not "unit" anymore. test_threading should be restricted to test the threading module: it should not test "indirectly" the logging module. If someone wants to test how logging behaves on fork, test_logging should get a new test. (I didn't check if it already has such test.) |
|
Hi, folks. |
|
While the number of imports reduced a lot, "import test.support" still imports 98 modules. That's a lot! IMO we can reduce it a little bit more :-) Examples of imports which could be lazy:
There are also tons of functions, it may be time to create even more submodules:
Move also move the following functions to socket_helper:
Serhiy: What do you think of these proposed submodules? |
|
OK, I continue to reduce the import module in test.supports. |
|
After PR20128, the import modules from 132 to 123. |
|
PR-21771 "Remove test helpers aliases in test.support" was just merged. It needs an immediate followup to document the new locations of constants and functions. The now dead entries in test.suppport doc must be moved into new support module docs, with whatever revisions. As of this moment, the module index has |
|
Hi, terry. When creating the new test helpers, the docs have been updated. |
|
PR21771 has broken a considerable amount of buildbopts so we would need to revert it if is not fixed in 24 hours |
Whops, sorry I just saw that this is being fixed here bpo-21785, apologies then! |
|
Sorry, I forgot that for the website, 3.x means 3.8, which has only 1 x_helper module. 3.9 has 3 and 3.10 has 7. |
|
The buildbots were fixed with PR-21785. '#' prefixes an issue on bpo, a PR on github. Confusing, especially now that PR #s are catching up to issue #s. |
|
# Confusing, especially now that PR #s are catching up to issue #s. |
|
test__osx_support and test_selectors are broken, examples: https://github.com/python/cpython/pull/21806/checks?check_run_id=966438645 2020-08-10T12:04:20.7613070Z ====================================================================== 2020-08-10T12:04:20.7617240Z Traceback (most recent call last):
2020-08-10T12:04:20.7617860Z File "/Users/runner/work/cpython/cpython/Lib/test/test__osx_support.py", line 23, in setUp
2020-08-10T12:04:20.7618560Z self.env = test.support.EnvironmentVarGuard()
2020-08-10T12:04:20.7621650Z AttributeError: module 'test.support' has no attribute 'EnvironmentVarGuard'and 2020-08-10T12:01:40.0736200Z ====================================================================== 2020-08-10T12:01:40.0744640Z Traceback (most recent call last):
2020-08-10T12:01:40.0747460Z File "/Users/runner/work/cpython/cpython/Lib/test/test_selectors.py", line 539, in test_register_bad_fd
2020-08-10T12:01:40.0753630Z bad_f = support.make_bad_fd()
2020-08-10T12:01:40.0757650Z AttributeError: module 'test.support' has no attribute 'make_bad_fd'Hai Shi: Would you mind to investigate this issue? |
|
|
Update. "import test.support" now imports 37 modules, instead of 171 previously. It's way better! 23:26:20 vstinner@apu$ ./python
Python 3.10.0a0 (heads/master:598a951844, Aug 7 2020, 17:24:10)
[GCC 10.2.1 20200723 (Red Hat 10.2.1-1)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> import test
>>> before=set(sys.modules)
>>> import test.support
>>> after=set(sys.modules)
>>> len(after - before)
37
>>> import pprint; pprint.pprint(sorted(after - before))
['_datetime',
'_elementtree',
'_heapq',
'_sysconfigdata_d_linux_x86_64-linux-gnu',
'_testcapi',
'_weakrefset',
'argparse',
'copy',
'datetime',
'difflib',
'fnmatch',
'gettext',
'heapq',
'math',
'pprint',
'pyexpat',
'pyexpat.errors',
'pyexpat.model',
'signal',
'sysconfig',
'test.support',
'test.support.testresult',
'traceback',
'unittest',
'unittest.case',
'unittest.loader',
'unittest.main',
'unittest.result',
'unittest.runner',
'unittest.signals',
'unittest.suite',
'unittest.util',
'weakref',
'xml',
'xml.etree',
'xml.etree.ElementPath',
'xml.etree.ElementTree']Shorter list if imports made by unittest are ignored: 23:27:42 vstinner@apu$ ./python
Python 3.10.0a0 (heads/master:598a951844, Aug 7 2020, 17:24:10)
[GCC 10.2.1 20200723 (Red Hat 10.2.1-1)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys, unittest, test
>>> before=set(sys.modules)
>>> before=set(sys.modules); import test.support; after=set(sys.modules)
>>> len(after) - len(before)
17
>>> import pprint; pprint.pprint(sorted(after - before))
['_datetime',
'_elementtree',
'_sysconfigdata_d_linux_x86_64-linux-gnu',
'_testcapi',
'copy',
'datetime',
'math',
'pyexpat',
'pyexpat.errors',
'pyexpat.model',
'sysconfig',
'test.support',
'test.support.testresult',
'xml',
'xml.etree',
'xml.etree.ElementPath',
'xml.etree.ElementTree'] |
|
Hi, victor:
|
If someone wants to continue the effort in libregrtest, I suggest to open a new issue. This one already has a long list of commits and messages about test.support. See test.libregrtest.runtest_mp module for the "multiprocessing" implementation which uses subprocess. It's used when you run tests using -jN option. Almost all buildbots use -jN: see make buildbottest. |
|
If someone wants to continue the effort in libregrtest, I suggest to open a new issue. |
|
oh, sorry, missing a word: I suggest this bpo.->I suggest close this bpo. |
|
Sadly, the work done on this issue is useless until test.regrtest imports less modules as well. So I created bpo-41718 follow-up issue: "test.regrtest has way too many imports". I consider that the work is done on test.support, so I close this issue. |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: