Why do I care aboutemail.utils? Well,email.utils is imported byemail.message, andemail.message is imported by lots of other stdlib modules:urllib.parse,mailbox, andimportlib.metadata. Improving the import time of this module has a cascading effect through lots of the rest of the standard library.

A lot of the import cost ofemail.utils has to do with themake_msgid function, which requires therandom andsocket modules. This function is used by many third-party libraries, but isn't used at all by any of the stdlib modules that importemail.utils. This patch moves the function into a separate submodule.

Issue:Improve import time of various stdlib modules #109653

AlexWaygood added2 commits

September 25, 2023 10:40

WIP

0aca553

news

f3c815d

AlexWaygood added type-feature

A feature request or enhancement

performance

Performance or resource usage

topic-email 3.13bugs and security fixes labels

Sep 25, 2023

AlexWaygood requested a review froma team as acode owner

September 25, 2023 10:03

bedevere-appbot added the awaiting review label

Sep 25, 2023

bedevere-appbot mentioned this pull request

Sep 25, 2023

Improve import time of various stdlib modules#109653

Closed

sobolevn reviewed

Sep 25, 2023

View reviewed changes

Misc/NEWS.d/next/Library/2023-09-25-10-47-22.gh-issue-109653.TUHrId.rst OutdatedShow resolvedHide resolved

I can spell my own name

992274f

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>

AA-Turner reviewed

Sep 25, 2023

View reviewed changes

Lib/email/_msgid.py OutdatedShow resolvedHide resolved

Lib/email/utils.py OutdatedShow resolvedHide resolved

AA-Turner reviewed

Sep 25, 2023

View reviewed changes

Lib/email/utils.py OutdatedShow resolvedHide resolved

Apply suggestions from code review

493aa33

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>

bitdancer reviewed

Sep 25, 2023

View reviewed changes

Lib/email/utils.py Outdated

		if attr == "make_msgid":
		from email._msgid import make_msgid
		return make_msgid
		raise AttributeError(f"module {__name__!r} has no attribute {attr!r}")

Copy link

Member

bitdancerSep 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

In general I don't like performance hacks like this, but they do have their place. I can't speak to whether or not this is a worthwhile performance hack, you should seek approval from the maintainers of the impacted modules for that.

That said, the stdlib itself makes no use of make_msgid, and email.utils is not itself considered part of the new email API. Moving it into a separate module and making that part of the non-legacy public API of the email module would actually make some sense. I guess we'd just call it 'msgid'? Then this code should issue a deprecation warning pointing to the new way to import make_msgid. It feels kind of weird to have a module with just one function, but there isn't really anything else related to it that I can think of. (I wonder...maybe make_msgid actually belongs in the UUID module? Probably not. Wrong RFC.)

Copy link

Contributor

hauntsaninja commentedOct 3, 2023

This feels a little strange to me. Should we instead just defer the imports? Might even be faster to do so

Copy link

MemberAuthor

AlexWaygood commentedOct 3, 2023•
edited
Loading

This feels a little strange to me. Should we instead just defer the imports? Might even be faster to do so

Yeah, that might be a better shout. Have been meaning to give it a go — just haven't got round to it yet :)

I'm not really enthused about the idea of a painful deprecation period in order to change the place where people are "meant" to import it from. This function is pretty widely used by third-party packages, and the improvement in import time doesn't seem worth the disruption to me :)

AlexWaygoodand others added5 commits

October 3, 2023 15:45

Merge branch 'main' into email-message-import

a697d9d

Abandon the separate submodule idea

03233f1

stats are slightly less impressive now because importing random is ch…

f11bc98

…eaper now

Merge branch 'email-message-import' ofhttps://github.com/AlexWaygood…

c5e7a61

…/cpython into email-message-import

Merge branch 'main' into email-message-import

bcaeb4c

Copy link

MemberAuthor

AlexWaygood commentedOct 3, 2023

I've updated the PR. I abandoned the idea of adding a new submodule; now I just defer the problematicrandom,socket andos imports to insidemake_msgid(). The reason I didn't do that originally was because I was concerned it would slow down themake_msgid() function, but local experimentation doesn't show that. (I ranpython -m timeit -s "from email.utils import make_msgid" "make_msgid()" both with and without the change to test.) Shows the importance of benchmarking!

AA-Turner reviewed

Oct 3, 2023

View reviewed changes

Lib/email/utils.pyShow resolvedHide resolved

Update Lib/email/utils.py

b387dcd

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>

hauntsaninja approved these changes

Oct 4, 2023

View reviewed changes

Copy link

Contributor

hauntsaninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Looks good

Lib/email/utils.py

		@@ -94,6 +88,8 @@ def formataddr(pair, charset='utf-8'):
		name.encode('ascii')
		except UnicodeEncodeError:
		if isinstance(charset, str):
		# lazy import to improve module import time
		from email.charset import Charset

Copy link

Contributor

hauntsaninjaOct 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Not sure I like this one, only fine with it sinceformataddr doesn't look too widely used (a lot of the time it's nice to pay these costs upfront, predictable performance is important, e.g. don't want the first request your webserver serves to be randomly slow)

Copy link

MemberAuthor

AlexWaygoodOct 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

(a lot of the time it's nice to pay these costs upfront, predictable performance is important

agreed. On the other hand, though, theemail package goes in quite heavily for lazy imports in some other places, so this does seem in keeping with that general philosophy:

cpython/Lib/email/__init__.py

Lines 28 to 61 inf786029

	# Some convenience routines. Don't import Parser and Message as side-effects
	# of importing email since those cascadingly import most of the rest of the
	# email package.
	defmessage_from_string(s,args,*kws):
	"""Parse a string into a Message object model.

	Optional _class and strict are passed to the Parser constructor.
	"""
	fromemail.parserimportParser
	returnParser(args,*kws).parsestr(s)

	defmessage_from_bytes(s,args,*kws):
	"""Parse a bytes string into a Message object model.

	Optional _class and strict are passed to the Parser constructor.
	"""
	fromemail.parserimportBytesParser
	returnBytesParser(args,*kws).parsebytes(s)

	defmessage_from_file(fp,args,*kws):
	"""Read a file and parse its contents into a Message object model.

	Optional _class and strict are passed to the Parser constructor.
	"""
	fromemail.parserimportParser
	returnParser(args,*kws).parse(fp)

	defmessage_from_binary_file(fp,args,*kws):
	"""Read a binary file and parse its contents into a Message object model.

	Optional _class and strict are passed to the Parser constructor.
	"""
	fromemail.parserimportBytesParser
	returnBytesParser(args,*kws).parse(fp)