email.message import time

@loewis

BPO	11454
Nosy	@loewis, @warsaw, @orsenthil, @ezio-melotti, @bitdancer, @serhiy-storchaka
Files	issue11454.diff issue11454_benchmarks.py email_import_speedup.patch

^{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 2013-06-26.16:09:07.209>
created_at = <Date 2011-03-09.20:07:16.685>
labels = ['library', 'performance']
title = 'email.message import time'
updated_at = <Date 2013-06-26.16:09:07.207>
user = 'https://bugs.python.org/rosslagerwall'

bugs.python.org fields:

activity = <Date 2013-06-26.16:09:07.207>
actor = 'r.david.murray'
assignee = 'none'
closed = True
closed_date = <Date 2013-06-26.16:09:07.209>
closer = 'r.david.murray'
components = ['Library (Lib)']
creation = <Date 2011-03-09.20:07:16.685>
creator = 'rosslagerwall'
dependencies = []
files = ['27201', '27225', '27226']
hgrepos = []
issue_num = 11454
keywords = ['patch']
message_count = 29.0
messages = ['130461', '130483', '130487', '130505', '170545', '170546', '170549', '170553', '170697', '170712', '170713', '170714', '170715', '170718', '170761', '170762', '170763', '170765', '170767', '170768', '170770', '170772', '171073', '171074', '171075', '171076', '171077', '191910', '191911']
nosy_count = 10.0
nosy_names = ['loewis', 'barry', 'orsenthil', 'nadeem.vawda', 'ezio.melotti', 'Arfrever', 'r.david.murray', 'rosslagerwall', 'python-dev', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'resource usage'
url = 'https://bugs.python.org/issue11454'
versions = ['Python 3.4']

Startup-time:

$ ./python -m timeit  -s 'import re'  're.compile("([^\ud800-\udbff]|\A)[\udc00-\udfff]([^\udc00-\udfff]|\Z)").search; re.purge()'
100 loops, best of 3: 4.16 msec per loop
$ ./python -m timeit  -s 'import re'  're.purge()'  're.compile("[\udc80-\udcff]").search'
100 loops, best of 3: 5.72 msec per loop
$ ./python -m timeit  'h=lambda s, p=set(map(chr, range(0xDC80, 0xDCFF+1))): any(c in p for c in s)'
10000 loops, best of 3: 60.5 usec per loop
$ ./python -m timeit  -s 'import re'  're.purge()'  're.compile("(?![^\udc80-\udcff])").search'
1000 loops, best of 3: 401 usec per loop
$ ./python -m timeit  -s 'import re'  're.purge()'  're.compile("[^\udc80-\udcff]*\Z").match'
1000 loops, best of 3: 427 usec per loop

Runtime:

$ ./python -m timeit  -s 'import re; h=re.compile("([^\ud800-\udbff]|\A)[\udc00-\udfff]([^\udc00-\udfff]|\Z)").search; s = "A"*1000'  'h(s)'
1000 loops, best of 3: 245 usec per loop
$ ./python -m timeit  -s 'import re; h=re.compile("[\udc80-\udcff]").search; s = "A"*1000'  'h(s)'
10000 loops, best of 3: 30.1 usec per loop
$ ./python -m timeit  -s 'h=lambda s, p=set(map(chr, range(0xDC80, 0xDCFF+1))): any(c in p for c in s); s = "A"*1000'  'h(s)'
10000 loops, best of 3: 164 usec per loop
$ ./python -m timeit  -s 'import re; h=re.compile("(?![^\udc80-\udcff])").search; s = "A"*1000'  'h(s)'
10000 loops, best of 3: 98.3 usec per loop
$ ./python -m timeit  -s 'import re; h=re.compile("[^\udc80-\udcff]*\Z").match; s = "A"*1000'  'h(s)'
10000 loops, best of 3: 34.6 usec per loop

Faster set-version:

$ ./python -m timeit  -s 'h=lambda s, hn=set(map(chr, range(0xDC80, 0xDD00))).isdisjoint: not hn(s); s = "A"*1000'  'h(s)'
10000 loops, best of 3: 43.8 usec per loop

Attached new benchmark file.

Results:

Testing runtime of the _has_surrogates functions
Generating chars...
Generating samples...
1.61 <- re.compile(current_regex).search
0.24 <- re.compile(short_regex).search
15.13 <- return any(c in surrogates for c in s)
10.21 <- for c in s: if c in surrogates: return True
0.85 <- return re.search(short_regex, s)
0.83 <- functools.partial(re.search, short_regex)
20.86 <- for c in map(ord, s): if c in range(0xDC80, 0xDCFF+1): return True
19.68 <- for c in map(ord, s): if 0xDC80 <= c <= 0xDCFF: return True
0.28 <- re.compile('[^\\udc80-\\udcff]*\Z').match
7.00 <- return not set(map(chr, range(0xDC80, 0xDCFF+1))).isdisjoint(s)

Testing startup time
0.57 <- r = re.compile('[\udc80-\udcff]').search
0.59 <- r = re.compile('[^\\udc80-\\udcff]\Z').match
199.79 <- r = re.compile('[\udc80-\udcff]').search; purge()
22.62 <- r = re.compile('[^\\udc80-\\udcff]\Z').match; purge()
1.12 <- r = pickle.loads(p)

So by your measurements the short search is the clear winner?

Yes, however it has a startup cost that the function that returns re.search(short_regex, s) and the one with functool.partial don't have, because with these the compilation happens at the first call.

If we use one of these two, the startup time will be reduced a lot, and the runtime will be ~2x faster.
If we use re.compile(short_regex).search the startup time won't be reduced as much, but the runtime will be ~8x faster.

Given that here we are trying to reduce the startup time and not the runtime, I think using one of those two functions is better.

Another possible solution to improve the startup time is trying to optimize _optimize_unicode -- not sure how much can be done there though.

This issue may be about reducing the startup time, but this function is a hot spot in the email package so I would prefer to sacrifice startup time optimization for an increase in speed.

However, given the improvements to import locking in 3.3, what about a self replacing function?

def _has_surrogates(s):
    import email.utils
    f = re.compile('[\udc80-\udcff]').search
    email.utils._has_surrogates = f
    return f(s)

That might work.

To avoid the overhead of the cache lookup I was thinking about something like

regex = None
def _has_surrogates(s):
    global regex
    if regex is None:
        regex = re.compile(short_regex)
    return regex.search(s)

but I have discarded it because it's not very pretty and still has the overhead of the function and an additional if. Your version solves both the problems in a more elegant way.

It passed the email test suite. Patch attached.

It would be better to add/improve the _has_surrogates tests before committing.
The patch I attached is also still valid if you want a further speed up improvement.

def _has_surrogates(s):
    try:
        s.encode()
        return False
    except UnicodeEncodeError:
        return True

Results:
0.26 <- re.compile(short_regex).search
0.06 <- try encode

I'm really not willing to inline any of those pre-compiled regular expressions. They are precompiled because for a program processing lots of email, they are hot spots. We could use the same "compile on demand" dodge on them, though.

Can you explain your changes to the

Woops.

Can you explain your changes to the ecre regex (keeping in mind that I don't know much about regex syntax).

Oh, yeah, and the encode benchmark is very instructive, thanks Serhiy :)

They are precompiled because for a program processing lots of email,
they are hot spots.

OK, I didn't know they were hot spots. Note that the regex are not recompiled everytime: they are compiled the first time and then taken from the cache (assuming they don't fall out from the bottom of the cache). This still has a small overhead though.

Can you explain your changes to the ecre regex (keeping in mind
that I don't know much about regex syntax).

-  (?P<charset>[^?]*?)   # non-greedy up to the next ? is the charset
+  (?P<charset>[^?]*)    # up to the next ? is the charset
   \?                    # literal ?
   (?P<encoding>[qb])    # either a "q" or a "b", case insensitive
   \?                    # literal ?
-  (?P<encoded>.*?)      # non-greedy up to the next ?= is the encoded string
+  (?P<encoded>[^?]*)    # up to the next ?= is the encoded string
   \?=                   # literal ?=

At the beginning, the non-greedy *? is unnecessary because [^?]* already stops at the first ? found.
The second change might actually be wrong if <encoded> is allowed to contain lone '?'s. The original regex used '.*?\?=', which means "match everything (including lone '?'s) until the first '?=')", mine means "match everything until the first '?'" which works fine as long as lone '?'s are not allowed.

Serhiy's suggestion is semantically different, but it might be still suitable if having _has_surrogate return True even for surrogates not in range \udc80-\udcff is OK.

Well, "other" surrogates will cause a different error later than with the current _has_surrogates logic, but it won't be any more mysterious than what would happen now, I think. Normally, if I understand correctly, other surrogates should never occur, so I don't think it is a real issue.

Yes, lone '?'s should not stop the pattern match in an encoded string. Even though I don't think they are normally supposed to occur, they do occur when encoded words are encoded incorrectly, and we get a better error recovery result if we look for ?= as the end.

New changeset 520490c4c388 by R David Murray in branch 'default':
bpo-11454: Reduce email module load time, improve surrogate check efficiency.
http://hg.python.org/cpython/rev/520490c4c388

I've checked in the encode version of the method. I'm going to pass on doing the other inlines, given that the improvement isn't that large. I will, however, keep the issue in mind as I make other changes to the code, and there will be a general performance review phase when I get done with the API additions/bug fixing in the email6 project.

Navigation Menu

Repository navigation

email.message import time #55663

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

email.message import time #55663

Description

Activity

serhiy-storchaka commented on Sep 19, 2012

serhiy-storchaka commented on Sep 19, 2012

ezio-melotti commented on Sep 19, 2012

bitdancer commented on Sep 19, 2012

ezio-melotti commented on Sep 19, 2012

bitdancer commented on Sep 19, 2012

ezio-melotti commented on Sep 19, 2012

bitdancer commented on Sep 19, 2012

ezio-melotti commented on Sep 19, 2012

serhiy-storchaka commented on Sep 19, 2012

bitdancer commented on Sep 23, 2012

bitdancer commented on Sep 23, 2012

bitdancer commented on Sep 23, 2012

ezio-melotti commented on Sep 23, 2012

bitdancer commented on Sep 23, 2012

python-dev commented on Jun 26, 2013

bitdancer commented on Jun 26, 2013

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions