Johnson9009 commented on code in PR #16357:
URL: https://github.com/apache/tvm/pull/16357#discussion_r1448670941


##########
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:
   even though the earlier code, there is only 1 function _serve_loop, the type 
of its 1st parameter is socket, we can only use 1 name "sock" or "conn", there 
is 2 caller of the function _serve_loop, just as you said, 1 caller's argument 
is a original socket, and another caller's argument is a result object from 
accept(), actually the type of the result object from accept still is socket, 
so I use the name "sock", because it closer with the type.



##########
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:
   Thanks for your explanation, I think just move the closure out of the 
function _serving will fix the issue, so from my side, it is better to keep the 
code simple, so please keep it as same as possible with the current 
implementation if there isn't reasonable requirement, thanks.



-- 
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]

Reply via email to