gh-102567: Add -X importtime=2 for logging an importtime message for already-loaded modules #118655

noahbkim · 2024-05-06T15:40:44Z

As mentioned in the issue:

While -X importtime is incredibly useful for analyzing module import times, by design, it doesn't log anything if an imported module has already been loaded. -X importtime=2 would provide additional output for every module that's already been loaded:

The updated example (-Ximporttime=2):

>>> import uuid
import time: cached    | cached     | builtins
import time: cached    | cached     | linecache
import time: cached    | cached     |   _io
import time: cached    | cached     |   os
import time: cached    | cached     |   sys
import time: cached    | cached     |   enum
import time:       594 |        594 |   _uuid
import time:      2428 |       3022 | uuid

With -Ximporttime:

>>> import uuid
import time:       351 |        351 |   _uuid
import time:       843 |       1194 | uuid

Discussion: https://discuss.python.org/t/x-importtrace-to-supplement-x-importtime-for-loaded-modules/23882/5
Prior email chain: https://mail.python.org/archives/list/python-ideas@python.org/thread/GEISYQ5BXWGKT33RWF77EOSOMMMFUBUS/

Issue: Add -X importtime=2 for additional logging when an imported module is already loaded #102567

Eclips4 · 2024-05-06T17:22:33Z

Hello!
This PR needs to add:

A NEWS entry
A whatsnew entry
A new paragraph in the Doc/cmdline for the -X option.
New test case in the Lib/test/test_cmd_line

noahbkim · 2024-05-06T18:32:13Z

Hello! This PR needs to add:

A NEWS entry

A whatsnew entry

A new paragraph in the Doc/cmdline for the -X option.

New test case in the Lib/test/test_cmd_line

Thanks for the quick feedback. I've committed a first pass of 1-3. As for tests: there appear to be no importtime tests (likely due to the fact that any simple implementation would be super noisy as other parts of the interpreter change). Would it be sufficient to do some rudimentary check i.e. import foo then ensure a corresponding foo row appears in the output?

Thanks again!

Eclips4 · 2024-05-06T19:34:47Z

Hello! This PR needs to add:

A NEWS entry

A whatsnew entry

A new paragraph in the Doc/cmdline for the -X option.

New test case in the Lib/test/test_cmd_line

Thanks for the quick feedback. I've committed a first pass of 1-3. As for tests: there appear to be no importtime tests (likely due to the fact that any simple implementation would be super noisy as other parts of the interpreter change). Would it be sufficient to do some rudimentary check i.e. import foo then ensure a corresponding foo row appears in the output?

Thanks again!

Yes, it would be sufficient :)

Eclips4 · 2024-05-06T19:51:26Z

Also, there's a merge conflict. Can you resolve it? (I can do it myself, but it seems like you have turned off this option)

noahbkim · 2024-05-07T17:43:11Z

Also, there's a merge conflict. Can you resolve it? (I can do it myself, but it seems like you have turned off this option)

Done, just added tests as well

noahbkim · 2024-05-28T18:09:29Z

@Eclips4 who should I ping to get final review? Is it alright if I @ the blamed reviewers?

Eclips4 · 2024-05-28T18:51:19Z

@Eclips4 who should I ping to get final review? Is it alright if I @ the blamed reviewers?

I guess @vstinner is the right person to review this 😄
However, you should move your changes from whatsnew/3.13.rst to whatsnew/3.14.rst since 3.13 branch is in feature-freeze mode, and this feature will appear only in the 3.14 version.

Eclips4 · 2024-05-28T20:09:25Z

However, you should move your changes from whatsnew/3.13.rst to whatsnew/3.14.rst since 3.13 branch is in feature-freeze mode, and this feature will appear only in the 3.14 version.

The same applies for the versionchanged directives.

serhiy-storchaka · 2024-09-05T06:20:44Z

Python/import.c

+     * imported module has already been loaded.
+     */
+    if (_PyInterpreterState_GetConfig(interp)->import_time >= 2) {
+#define import_level FIND_AND_LOAD(interp).import_level


Why use a macro here?

I wanted to mirror line 3652 (import_find_and_load) as closely as possible--since it's only used once I can get rid of the macro if you prefer.

Python/initconfig.c

serhiy-storchaka · 2024-09-05T06:24:47Z

Lib/test/test_cmd_line.py

+        res = assert_python_ok('-X', 'importtime', '-c', code)
+        self.assertRegex(
+            res.err.decode("utf-8"),
+            r"import time: \s*\d+ | \s*\d+ | \s*os"


It would be nice to check that this line occurs only once.

You can also check that the line with "cached" is not output.

I've added a check to ensure -Ximporttime does not include =2 output. I added a rudimentary check that -Ximporttime only shows os once, though this may be flaky if the structure of the os module changes.

Python/import.c

Python/initconfig.c

Doc/using/cmdline.rst

ghost · 2024-09-30T22:02:52Z

All commit authors signed the Contributor License Agreement.

noahbkim · 2024-09-30T22:16:21Z

Apologies for the noise, I had to amend some of the commits with the correct email to pick up the right CLA.

noahbkim · 2024-11-04T21:15:25Z

Pinging @serhiy-storchaka for re-review; I intend to leave the mypy errors as-is because this import try/except mechanism is already used elsewhere in the stdlib.

noahbkim · 2025-01-10T19:10:33Z

LGTM! Before merging, we need to know @pablogsal and @ambv opinion on change in Lib/_pyrepl/unix_console.py

Re-pinging for quick review!

picnixz · 2025-01-11T00:55:35Z

@noahbkim Can you make the CI pass on Windows and macOS? the logs are no more available so I can't see what happened.

noahbkim · 2025-01-13T19:04:59Z

All checks except for mypy should pass. The issue reported by mypy is the following idiom

try:
    import posix
except ImportError:
    posix = None

Since this is used pretty widely throughout the standard library, I figured it should be left as is and the warning ignored. I could suppress the warning instead.

Update: woah, I lied, something has changed since my last push. I'll fix these failures today.

picnixz · 2025-01-13T19:47:00Z

Since this is used pretty widely throughout the standard library, I figured it should be left as is and the warning ignored. I could suppress the warning instead.

You can mypy-ignore it I think. Otherwise, future modifications in that file would also be flagged, right?

Lib/_pyrepl/unix_console.py

Doc/whatsnew/3.14.rst

Lib/_pyrepl/unix_console.py

Doc/using/cmdline.rst

python-cla-bot · 2025-04-14T17:24:07Z

All commit authors signed the Contributor License Agreement.

Doc/using/cmdline.rst

Doc/whatsnew/3.14.rst

noahbkim · 2025-05-01T17:44:33Z

@AA-Turner merged your changes, happy to do anything else necessary, apologies for the inconvenience of the organization branch. Not sure where your comment went (I saw it in my email), but thanks for helping push this through.

AA-Turner · 2025-05-01T17:46:44Z

Thanks @noahbkim, no worries! Are you able to make the changes to config_set_import_time etc to reserve values >=2?

We also recently changed our CLA bot, please can you click on the red 'not signed' and ensure everything is up to date?

A

noahbkim · 2025-05-01T19:22:36Z

I think I've messed up my branch, I'm going to attempt to squash everything so I can overwrite my commit email correctly.

AA-Turner · 2025-05-01T20:20:45Z

Ok, please ping for a review when you've sorted the branch out! If it's easier, feel free to just open a new PR, but a force push here should also work.

A

noahbkim · 2025-05-01T20:29:14Z

@AA-Turner I've squashed my changes and given everything a second pass.

It looks like I caused some kind of regression in test_embed.py, but none of the expected invariants make sense to me (I'm not sure why we expect import_time to be set to 1 in any of the listed circumstances). Looking through the blame was not particularly illuminating. My suspicion is that these tests have been migrated/updated several times without regard for what they're actually testing (or perhaps implicit behavior surrounding -Ximporttime has changed since the test suite's addition?).

Zeroing import_time in said cases returns everything to passing, but a second pair of eyes on that particular bit would be greatly appreciated.

I have about 6 tests failing locally, one of which prevents me from compiling with --enable-optimizations (test_json) but I can't imagine those are related. I'll check if my merge base is behind + fix the CI lint issue.

…ded modules

chris-eibl · 2025-05-02T05:09:40Z

Lib/_pyrepl/unix_console.py

-        except ImportError:
-            return None
-        if posix._is_inputhook_installed():
+        # avoid inline imports here so the repl doesn't get flooded with import


A similar change might be needed for WindowsConsole.input_hook:

cpython/Lib/_pyrepl/windows_console.py

Lines 236 to 243 in b8633f9

@property

def input_hook(self):

try:

import nt

except ImportError:

return None

if nt._is_inputhook_installed():

return nt._inputhook

noahbkim requested review from kumaraditya303 and ericsnowcurrently as code owners May 6, 2024 15:40

bedevere-app bot added the awaiting review label May 6, 2024

bedevere-app bot mentioned this pull request May 6, 2024

Add -X importtime=2 for additional logging when an imported module is already loaded #102567

Open

noahbkim requested review from sethmlarson, brettcannon, freakboy3742, erlend-aasland, jaraco, barneygale, a team, corona10, ethanfurman, JelleZijlstra, AlexWaygood, terryjreedy, pfmoore, pradyunsg, ericvsmith, rhettinger and encukou as code owners July 11, 2024 16:26

serhiy-storchaka reviewed Sep 5, 2024

View reviewed changes

Doc/using/cmdline.rst Show resolved Hide resolved

noahbkim force-pushed the feature/import-cache-2 branch 2 times, most recently from 4f627c5 to 0281267 Compare September 30, 2024 22:15

noahbkim force-pushed the feature/import-cache-2 branch from 80aba50 to 0281267 Compare September 30, 2024 22:26

picnixz requested review from serhiy-storchaka, pablogsal and ambv January 11, 2025 00:49

picnixz reviewed Jan 13, 2025

View reviewed changes

Lib/_pyrepl/unix_console.py Show resolved Hide resolved

AA-Turner reviewed Apr 1, 2025

View reviewed changes

Doc/whatsnew/3.14.rst Show resolved Hide resolved

Lib/_pyrepl/unix_console.py Show resolved Hide resolved

AA-Turner reviewed Apr 7, 2025

View reviewed changes

Doc/using/cmdline.rst Outdated Show resolved Hide resolved

noahbkim force-pushed the feature/import-cache-2 branch from 60d94b3 to 2abb3d8 Compare April 14, 2025 17:25

AA-Turner reviewed May 1, 2025

View reviewed changes

Doc/using/cmdline.rst Outdated Show resolved Hide resolved

Doc/using/cmdline.rst Outdated Show resolved Hide resolved

Doc/using/cmdline.rst Outdated Show resolved Hide resolved

Doc/using/cmdline.rst Outdated Show resolved Hide resolved

Doc/whatsnew/3.14.rst Show resolved Hide resolved

noahbkim force-pushed the feature/import-cache-2 branch from e53dea1 to 0ed040c Compare May 1, 2025 20:24

Add -X importtime=2 for logging an importtime message for already-loa…

62d09cd

…ded modules

noahbkim force-pushed the feature/import-cache-2 branch from 0ed040c to 62d09cd Compare May 1, 2025 20:31

chris-eibl reviewed May 2, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-102567: Add -X importtime=2 for logging an importtime message for already-loaded modules #118655

gh-102567: Add -X importtime=2 for logging an importtime message for already-loaded modules #118655

noahbkim commented May 6, 2024 •

edited

Loading

Eclips4 commented May 6, 2024 •

edited

Loading

noahbkim commented May 6, 2024

Eclips4 commented May 6, 2024

Eclips4 commented May 6, 2024

noahbkim commented May 7, 2024

noahbkim commented May 28, 2024

Eclips4 commented May 28, 2024

Eclips4 commented May 28, 2024 •

edited

Loading

serhiy-storchaka Sep 5, 2024

noahbkim Sep 30, 2024

serhiy-storchaka Sep 5, 2024

noahbkim Sep 30, 2024 •

edited

Loading

ghost commented Sep 30, 2024 •

edited by ghost

Loading

noahbkim commented Sep 30, 2024

noahbkim commented Nov 4, 2024

noahbkim commented Jan 10, 2025

picnixz commented Jan 11, 2025

noahbkim commented Jan 13, 2025 •

edited

Loading

picnixz commented Jan 13, 2025

python-cla-bot bot commented Apr 14, 2025 •

edited

Loading

noahbkim commented May 1, 2025

AA-Turner commented May 1, 2025 •

edited

Loading

noahbkim commented May 1, 2025

AA-Turner commented May 1, 2025

noahbkim commented May 1, 2025

chris-eibl May 2, 2025

	@property
	def input_hook(self):
	try:
	import nt
	except ImportError:
	return None
	if nt._is_inputhook_installed():
	return nt._inputhook

gh-102567: Add -X importtime=2 for logging an importtime message for already-loaded modules #118655

Are you sure you want to change the base?

gh-102567: Add -X importtime=2 for logging an importtime message for already-loaded modules #118655

Conversation

noahbkim commented May 6, 2024 • edited Loading

Eclips4 commented May 6, 2024 • edited Loading

noahbkim commented May 6, 2024

Eclips4 commented May 6, 2024

Eclips4 commented May 6, 2024

noahbkim commented May 7, 2024

noahbkim commented May 28, 2024

Eclips4 commented May 28, 2024

Eclips4 commented May 28, 2024 • edited Loading

serhiy-storchaka Sep 5, 2024

Choose a reason for hiding this comment

noahbkim Sep 30, 2024

Choose a reason for hiding this comment

serhiy-storchaka Sep 5, 2024

Choose a reason for hiding this comment

noahbkim Sep 30, 2024 • edited Loading

Choose a reason for hiding this comment

ghost commented Sep 30, 2024 • edited by ghost Loading

noahbkim commented Sep 30, 2024

noahbkim commented Nov 4, 2024

noahbkim commented Jan 10, 2025

picnixz commented Jan 11, 2025

noahbkim commented Jan 13, 2025 • edited Loading

picnixz commented Jan 13, 2025

python-cla-bot bot commented Apr 14, 2025 • edited Loading

noahbkim commented May 1, 2025

AA-Turner commented May 1, 2025 • edited Loading

noahbkim commented May 1, 2025

AA-Turner commented May 1, 2025

noahbkim commented May 1, 2025

chris-eibl May 2, 2025

Choose a reason for hiding this comment

noahbkim commented May 6, 2024 •

edited

Loading

Eclips4 commented May 6, 2024 •

edited

Loading

Eclips4 commented May 28, 2024 •

edited

Loading

noahbkim Sep 30, 2024 •

edited

Loading

ghost commented Sep 30, 2024 •

edited by ghost

Loading

noahbkim commented Jan 13, 2025 •

edited

Loading

python-cla-bot bot commented Apr 14, 2025 •

edited

Loading

AA-Turner commented May 1, 2025 •

edited

Loading