-
Notifications
You must be signed in to change notification settings - Fork 6.1k
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
[core][1/N] Make executor to be a long-running Python thread #51016
Conversation
|
I messed up the commits in the old PR, so I created a new one. Additionally, this comment contains details of the investigation I conducted on PyGILState_STATE. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
…ject#51016) This is the same as ray-project#50644 Currently, tasks are sometimes executed on the main thread and sometimes on other threads which is created by C++. However, C++ threads are only considered Python threads when they execute Python functions. Hence, when a task finishes, the thread-local state will be garbage-collected. See ray-project#46336 (comment) for more details. This PR uses the CPython API to treat C++ threads as Python threads, even if they do not execute Python functions. https://docs.python.org/3/c-api/init.html#non-python-created-threads This PR handles synchronous tasks. After this PR is merged, the follow-up tasks are: * Always create a default executor. * Move the constructor to the default executor. * Support asynchronous tasks. ## Related issue number Part of ray-project#46336 --------- Signed-off-by: kaihsun <kaihsun@anyscale.com> Signed-off-by: vs030455 <vamshikdshetty@gmail.com>
…ject#51016) This is the same as ray-project#50644 Currently, tasks are sometimes executed on the main thread and sometimes on other threads which is created by C++. However, C++ threads are only considered Python threads when they execute Python functions. Hence, when a task finishes, the thread-local state will be garbage-collected. See ray-project#46336 (comment) for more details. This PR uses the CPython API to treat C++ threads as Python threads, even if they do not execute Python functions. https://docs.python.org/3/c-api/init.html#non-python-created-threads This PR handles synchronous tasks. After this PR is merged, the follow-up tasks are: * Always create a default executor. * Move the constructor to the default executor. * Support asynchronous tasks. ## Related issue number Part of ray-project#46336 --------- Signed-off-by: kaihsun <kaihsun@anyscale.com>
…ject#51016) This is the same as ray-project#50644 Currently, tasks are sometimes executed on the main thread and sometimes on other threads which is created by C++. However, C++ threads are only considered Python threads when they execute Python functions. Hence, when a task finishes, the thread-local state will be garbage-collected. See ray-project#46336 (comment) for more details. This PR uses the CPython API to treat C++ threads as Python threads, even if they do not execute Python functions. https://docs.python.org/3/c-api/init.html#non-python-created-threads This PR handles synchronous tasks. After this PR is merged, the follow-up tasks are: * Always create a default executor. * Move the constructor to the default executor. * Support asynchronous tasks. ## Related issue number Part of ray-project#46336 --------- Signed-off-by: kaihsun <kaihsun@anyscale.com>
Why are these changes needed?
This is the same as #50644
Currently, tasks are sometimes executed on the main thread and sometimes on other threads which is created by C++. However, C++ threads are only considered Python threads when they execute Python functions. Hence, when a task finishes, the thread-local state will be garbage-collected. See #46336 (comment) for more details.
This PR uses the CPython API to treat C++ threads as Python threads, even if they do not execute Python functions.
https://docs.python.org/3/c-api/init.html#non-python-created-threads
This PR handles synchronous tasks. After this PR is merged, the follow-up tasks are:
Related issue number
Part of #46336
Checks
git commit -s
) in this PR.scripts/format.sh
to lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/
under thecorresponding
.rst
file.