Switch threading model to fork on macOS
#8347
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This patch switches the threading model on macOS to `fork` instead of `spawn`. It also includes a check that the environment variable OBJC_DISABLE_INITIALIZE_FORK_SAFETY=YES to work properly.
|
@leandron @AndrewZhaoLuo following up on our discussion here, https://discuss.tvm.apache.org/t/why-is-auto-tuning-with-resnet-failing-at-task-1, a proposal for updating tvmc to work on macOS. |
leandron
reviewed
Jun 28, 2021
There was a problem hiding this comment.
I understand this will fix the immediate problem reported. I just wonder whether this shouldn't be fixed at import tvm level, considering the usages via the Python API.
In case there agreement to fix this here, I'm also happy to take this PR.
also cc @comaniac
|
Hmm I personally don't think this is the right move. This has serious
sideffects on program behavior.
For example, if a user is running other scripts besides TVM in the same
session it could cause unexpected behavior. I think the best thing is
having the users manually change things and we focus on giving the user a
proper error message if we detect this error is applicable.
…On Mon, Jun 28, 2021, 2:44 AM Leandro Nunes ***@***.***> wrote:
***@***.**** commented on this pull request.
I understand this will fix the immediate problem reported. I just wonder
whether this shouldn't be fixed at import tvm level, considering the
usages via the Python API.
In case there agreement to fix this here, I'm also happy to take this PR.
also cc @comaniac <https://github.com/comaniac>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#8347 (review)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADJWVW7BUEVQKG34UTG4M6DTVA77XANCNFSM47KU3X7Q>
.
|
|
Yes, it's my understanding that the appropriate solution is to switch to using methods like POpenWorker, like in this patch. #7889. I'm going to close this PR, but we should continue the discussion. |
|
Im going to spend a little bit today understanding what needs to change to
make spawn work. I think just some objects need to be able to be pickle
able.
…On Mon, Jun 28, 2021, 9:24 AM Andrew Luo ***@***.***> wrote:
Hmm I personally don't think this is the right move. This has serious
sideffects on program behavior.
For example, if a user is running other scripts besides TVM in the same
session it could cause unexpected behavior. I think the best thing is
having the users manually change things and we focus on giving the user a
proper error message if we detect this error is applicable.
On Mon, Jun 28, 2021, 2:44 AM Leandro Nunes ***@***.***>
wrote:
> ***@***.**** commented on this pull request.
>
> I understand this will fix the immediate problem reported. I just wonder
> whether this shouldn't be fixed at import tvm level, considering the
> usages via the Python API.
>
> In case there agreement to fix this here, I'm also happy to take this PR.
>
> also cc @comaniac <https://github.com/comaniac>
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#8347 (review)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ADJWVW7BUEVQKG34UTG4M6DTVA77XANCNFSM47KU3X7Q>
> .
>
|
|
Ok, so the actual tuning system is built with duct tape, but I think the minimum changes to make this work is actually easier than initially thought: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch switches the threading model on macOS to
forkinsteadof
spawn. It also includes a check that the environment variableOBJC_DISABLE_INITIALIZE_FORK_SAFETY=YES to work properly.