Title: SRE engine should release the GIL when/if possible
Type: enhancement Stage:
Components: Extension Modules Versions: Python 3.8
Status: open Resolution:
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: barry, effbot, eric_noyau, georg.brandl, loewis, serhiy.storchaka
Priority: normal Keywords:

Created on 2005-11-25 13:57 by eric_noyau, last changed 2018-05-25 18:58 by barry.

File name Uploaded Description Edit
regex.diff eric_noyau, 2005-11-28 14:11 updated patch for _sre.c and sre.h
Messages (10)
msg49122 - (view) Author: Eric Noyau (eric_noyau) Date: 2005-11-25 13:57
In a multi-threaded program that does lots of regular
expression searching, some of them on very long strings
with complex regex we've noticed that everything stops
when a regular expression is searching.

One of the issue is that the re engine does not release
the interpreter lock while it is running. All the
other threads are therefore blocked for the entire time
it takes to do the regular expression search.

See the thread in python-dev about it:

msg49123 - (view) Author: Eric Noyau (eric_noyau) Date: 2005-11-25 14:02
Logged In: YES 

I'm attaching a diff to this bug that remove this limitation
if it sane to do so. If a search is done on a string or a
unicode object (which by definition are immutable) the GIL
is released and reacquired everytime a search is done.

I've tested this patch in both a simple tests (start a
thread with a greedy regex on a monstruous string and verify
that the othe python threads are still active) and by
running our internal application verifying that nothing is
blocking anymore.
msg49124 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2005-11-25 21:38
Logged In: YES 

The patch looks good, but I wonder if it is safe.  The SRE_STATE structure that SRE_SEARCH_INNER uses is potentially visible to the application-level Python code, via the (undocumented) scanner objects:

>>> r = re.compile(r"hello")
>>> s = r.scanner("big string in which to search")
<_sre.SRE_Match object at 0x12345678>

Each call to continues the previous search with the same SRE_STATE.  The problem with releasing the GIL as you do is that several threads could call concurrently, which would most probably crash CPython.

This probably means that you need to add a lock in SRE_STATE and acquire it while searching, to serialize its usage.  Of course, we should then be careful about what overhead this gives to applications that use regexps on a lot of small strings...

Another note: for consistency, match() should also release the GIL if search() does.
msg49125 - (view) Author: Eric Noyau (eric_noyau) Date: 2005-11-28 14:11
Logged In: YES 

Thanks for your comments. I've updated the patch to fix your
issues, but without introducing a per-state object lock.

What I did instead is to mark a state as not supporting
concurrency when a scanner object creates it. So the GIL
will not be released for scanners objects at all.

For consistency match also release the GIL now, if possible.
msg49126 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2006-02-18 23:41
Logged In: YES 

Fredrik, do you have time to review this?
msg49127 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2006-11-25 15:13
I believe the patch is incorrect. While matching, sre may allocate memory through Python API, and it may raise exceptions through Python API. Neither is allowed when the GIL is released

Tentatively rejecting the patch.

Eric, if you think the patch is correct or can be corrected, please update it to the current subversion trunk.
msg49128 - (view) Author: Eric Noyau (eric_noyau) Date: 2006-11-27 13:03
Albeit I still think releasing the GIL during regex matching would be beneficial, I agree with Martin that the patch is not good enough for that purpose. I was not aware of the requirement to hold the GIL in order to do memory allocation.

Anyway, since implementing this patch, we have reviewed our usage of regex and supressed the really greedy ones. As such this patch is no longer needed by us either. It would probably make our application a tiny fractional bit faster, but not the order of magnitude faster than we experienced before removing the big regexes.

In conclusion I thank Martin for the review as I've learned something new, and instead of trying to do a more fine grained fix I'm closing this bug as the current behaviour is good enough if you avoid using stupid regexes...

msg317692 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2018-05-25 17:31
I'm reopening this bug even though it's been long closed and even though the attached patch is no longer relevant.

We recently found a degenerative performance problem with entrypoints and threads.  As the number of threads increases, it becomes increasingly horrible to extract entrypoints.

The problem is that entrypoints uses glob.iglob() to find the .{dist,egg}-info directory.  iglob() uses regexps on a fairly simple pattern.  GIL contention can cause entrypoint discovery with 16 threads to take ~10 seconds for all threads to complete.

We have some test cases that I'll try to attach later, but since my bugs archeology found this bug, I decided to reopen it rather than start with a new bug.
msg317700 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-05-25 18:54
It is possible to implement this feature, but it has some limitations.

Did you try to use regex which has this feature? For short strings and simple patterns there may be no benefit.
msg317701 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2018-05-25 18:58
> Did you try to use regex which has this feature? For short strings and simple patterns there may be no benefit.

In my use case, it’s not possible, since the problematic API is glob.iglob() through multiple levels of calls.
Date User Action Args
2018-05-25 18:58:39barrysetmessages: + msg317701
2018-05-25 18:54:33serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg317700

assignee: serhiy.storchaka
type: enhancement
2018-05-25 18:00:17arigosetnosy: - arigo
2018-05-25 17:31:03barrysetstatus: closed -> open

assignee: effbot -> (no value)
keywords: - patch
title: SRE engine do not release the GIL -> SRE engine should release the GIL when/if possible
nosy: + barry
versions: + Python 3.8
messages: + msg317692
components: + Extension Modules, - None
resolution: rejected ->
2005-11-25 13:57:15eric_noyaucreate