Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-40010: Optimize signal handling in multithreaded applications #19067

Merged
merged 1 commit into from
Mar 19, 2020
Merged

bpo-40010: Optimize signal handling in multithreaded applications #19067

merged 1 commit into from
Mar 19, 2020

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Mar 19, 2020

If a thread gets a signal, the bytecode evaluation loop no longer
tries to handle signals before executing each bytecode instruction,
if the thread must not handle signals.

Previously, it was was done until the main interpreter had the
opportunity to handle signals.

COMPUTE_EVAL_BREAKER() and SIGNAL_PENDING_SIGNALS() no longer set
eval_breaker to 1 if the current thread must not handle signals.

https://bugs.python.org/issue40010

@vstinner
Copy link
Member Author

I merged PR #19066 which makes https://bugs.python.org/issue40010 worse for subinterpreters if I understand correctly. This PR should make subinterpreter as efficient as they were, or even more efficient, to handle signals.

@@ -120,14 +120,23 @@ static size_t opcache_global_hits = 0;
static size_t opcache_global_misses = 0;
#endif


static int
thread_can_handle_signals(void)
Copy link
Contributor

@aeros aeros Mar 19, 2020

Choose a reason for hiding this comment

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

The following seems a bit more specifically descriptive of what it's doing, and would make more sense if it's ever used in the future for something other than signal handling:

Suggested change
thread_can_handle_signals(void)
current_is_main_thread(void)

Copy link
Member Author

Choose a reason for hiding this comment

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

Even if only the main thread of the main interpreter can handle signals, I prefer "thread_can_handle_signals" name to make the function intent more explicit.

@aeros aeros added the performance Performance or resource usage label Mar 19, 2020
@vstinner
Copy link
Member Author

macOS failed

test.pythoninfo failed with:

ERROR: collect_pwd() failed
Traceback (most recent call last):
  File "/Users/runner/runners/2.165.2/work/cpython/cpython/Lib/test/pythoninfo.py", line 761, in collect_info
    collect_func(info_add)
  File "/Users/runner/runners/2.165.2/work/cpython/cpython/Lib/test/pythoninfo.py", line 336, in collect_pwd
    groups = os.getgrouplist(entry.pw_name, entry.pw_gid)
OSError: [Errno 0] Error

test_ttk_guionly failed on Ubuntu:

(...)
test_initialization_no_master (tkinter.test.test_ttk.test_extensions.LabeledScaleTest) ... ok
test_resize (tkinter.test.test_ttk.test_extensions.LabeledScaleTest) ... ok
test_variable_change (tkinter.test.test_ttk.test_extensions.LabeledScaleTest) ... Timeout (0:20:00)!
Thread 0x00007fa8861ec080 (most recent call first):
  File "/home/runner/work/cpython/cpython/Lib/tkinter/__init__.py", line 697 in wait_visibility
  File "/home/runner/work/cpython/cpython/Lib/tkinter/test/test_ttk/test_extensions.py", line 147 in test_variable_change
  File "/home/runner/work/cpython/cpython/Lib/unittest/case.py", line 616 in _callTestMethod
(...)

These two errors are unrelated to my PR.

If a thread different than the main thread gets a signal, the
bytecode evaluation loop is no longer interrupted at each bytecode
instruction to check for pending signals which cannot be handled.
Only the main thread of the main interpreter can handle signals.

Previously, the bytecode evaluation loop was interrupted at each
instruction until the main thread handles signals.

Changes:

* COMPUTE_EVAL_BREAKER() and SIGNAL_PENDING_SIGNALS() no longer set
  eval_breaker to 1 if the current thread cannot handle signals.
* take_gil() now always recomputes eval_breaker.
@vstinner vstinner merged commit 5a3a71d into python:master Mar 19, 2020
@vstinner vstinner deleted the signal_pending_signals branch March 19, 2020 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance or resource usage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants