-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[RPC] Fix tuning on macOS and Windows (#15771) #16357
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
Conversation
3197d51 to
bf41f88
Compare
Fix regression in (apache#15187) when multiprocessing start method is not 'fork', which prevented tuning from working. This affects macOS and Windows. Also in python 3.14 the default start method will be 'spawn'.
bf41f88 to
6e61c39
Compare
|
CC @Johnson9009 |
Johnson9009
left a comment
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.
sorry for late review, threre are some questions.
python/tvm/rpc/server.py
Outdated
| """Server loop""" | ||
| sockfd = sock.fileno() | ||
| temp = _server_env(load_library, work_path) | ||
| _ffi_api.ServerLoop(sockfd) |
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.
can the sockfd be inline?
python/tvm/rpc/server.py
Outdated
|
|
||
|
|
||
| def _serve_loop(sock, load_library, work_path=None): | ||
| """Server loop""" |
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.
del the meaningless comment because it give no more information beyond the function name, or write a meaning one.
|
The changes from the original code are now minimal, just removing the closure. |
Johnson9009
left a comment
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.
Thanks, look good to me.
Fix regression in (#15187) when multiprocessing start method is not 'fork', which prevented tuning from working. This affects macOS and Windows. Also in python 3.14 the default start method will be 'spawn'.