Johnson9009 commented on code in PR #16357:
URL: https://github.com/apache/tvm/pull/16357#discussion_r1448172941
##########
python/tvm/rpc/server.py:
##########
@@ -119,6 +119,15 @@ def download_linked_module(file_name):
return temp
+def _serve_loop(sock, load_library, work_path=None):
+ """Server loop"""
+ sockfd = sock.fileno()
+ temp = _server_env(load_library, work_path)
+ _ffi_api.ServerLoop(sockfd)
Review Comment:
can the sockfd be inline?
##########
python/tvm/rpc/server.py:
##########
@@ -119,6 +119,15 @@ def download_linked_module(file_name):
return temp
+def _serve_loop(sock, load_library, work_path=None):
Review Comment:
can we keep the parameter name consistent, like use " conn" all or use "
sock" all?
Is there requirements let work_path be a optional parameter?
##########
python/tvm/rpc/server.py:
##########
@@ -119,6 +119,15 @@ def download_linked_module(file_name):
return temp
+def _serve_loop(sock, load_library, work_path=None):
+ """Server loop"""
Review Comment:
del the meaningless comment because it give no more information beyond the
function name, or write a meaning one.
##########
python/tvm/rpc/server.py:
##########
@@ -119,6 +119,15 @@ def download_linked_module(file_name):
return temp
+def _serve_loop(sock, load_library, work_path=None):
+ """Server loop"""
+ sockfd = sock.fileno()
+ temp = _server_env(load_library, work_path)
+ _ffi_api.ServerLoop(sockfd)
+ if not work_path:
+ temp.remove()
Review Comment:
curious, why need the temp? it seems this function only have one caller, and
it will give the work_path.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
