Lunderberg commented on code in PR #10909:
URL: https://github.com/apache/tvm/pull/10909#discussion_r849650057
##########
python/tvm/contrib/hexagon/build.py:
##########
@@ -356,26 +358,46 @@ def _copy_binaries(self):
for item in self.ANDROID_HEXAGON_RPC_FILES:
self._copy_to_remote(lib_dir / item, self._workspace / item)
+ def _process_forwarded_ports(self):
+ forwarded_ports = subprocess.check_output(self._adb_device_sub_cmd +
["forward", "--list"])
+ existing_forwards = []
+ for forward in str(forwarded_ports).split("\\n"):
+ entry = forward.split()
+ if len(entry) == 3:
+ _, local, _ = entry
+ existing_forwards.append(int(local.strip("tcp:")))
+ return existing_forwards
+
+ def _forward_ports(self, rpc_server_port, existing_forwards):
+ # Enable port forward for RPC server. We forward the first ten open
ports
+ # starting from the rpc_server_port
+ port = rpc_server_port
+ while len(self.forwarded_ports_) < 10:
Review Comment:
I believe the total number of ports isn't important, instead trying to match
the port range that may be attempted a server, when it searches for an
available port to listen on. How often is the port already forwarded or in
use, and would be be better to throw an error in those cases?
##########
python/tvm/contrib/hexagon/build.py:
##########
@@ -356,26 +358,46 @@ def _copy_binaries(self):
for item in self.ANDROID_HEXAGON_RPC_FILES:
self._copy_to_remote(lib_dir / item, self._workspace / item)
+ def _process_forwarded_ports(self):
Review Comment:
Nit: Rename to `_existing_forwarded_ports` or `_get_forwarded_ports`. The
current name reads as if this is performing some processing on each forwarded
port.
##########
python/tvm/contrib/hexagon/build.py:
##########
@@ -356,26 +358,46 @@ def _copy_binaries(self):
for item in self.ANDROID_HEXAGON_RPC_FILES:
self._copy_to_remote(lib_dir / item, self._workspace / item)
+ def _process_forwarded_ports(self):
+ forwarded_ports = subprocess.check_output(self._adb_device_sub_cmd +
["forward", "--list"])
+ existing_forwards = []
+ for forward in str(forwarded_ports).split("\\n"):
+ entry = forward.split()
+ if len(entry) == 3:
+ _, local, _ = entry
+ existing_forwards.append(int(local.strip("tcp:")))
+ return existing_forwards
+
+ def _forward_ports(self, rpc_server_port, existing_forwards):
+ # Enable port forward for RPC server. We forward the first ten open
ports
+ # starting from the rpc_server_port
+ port = rpc_server_port
+ while len(self.forwarded_ports_) < 10:
+ if port not in existing_forwards and not _is_port_in_use(port):
+ subprocess.check_call(
+ self._adb_device_sub_cmd + ["forward", f"tcp:{port}",
f"tcp:{port}"]
+ )
+ self.forwarded_ports_.append(port)
+ port += 1
+
+ def _reverse_ports(self, rpc_tracker_port):
+ subprocess.check_call(
+ self._adb_device_sub_cmd
+ + ["reverse", f"tcp:{rpc_tracker_port}", f"tcp:{rpc_tracker_port}"]
+ )
+
def _run_server_script(self):
"""Setup the ADB connection and execute the server script."""
- # Removed pre-defined forward/reverse rules
- subprocess.check_call(self._adb_device_sub_cmd + ["forward",
"--remove-all"])
- subprocess.check_call(self._adb_device_sub_cmd + ["reverse",
"--remove-all"])
-
+ # Collect any existing adb port forwarding to avoid duplication
+ # with another running process
+ existing_forwards = self._process_forwarded_ports()
# Enable port reverse for RPC tracker
rpc_tracker_port = self._rpc_info["rpc_tracker_port"]
rpc_server_port = self._rpc_info["rpc_server_port"]
- subprocess.check_call(
- self._adb_device_sub_cmd
- + ["reverse", f"tcp:{rpc_tracker_port}", f"tcp:{rpc_tracker_port}"]
- )
- # Enable port forward for RPC server. We forward 9 ports after the
rpc_server_port.
- for i in range(0, 10):
- subprocess.check_call(
- self._adb_device_sub_cmd
- + ["forward", f"tcp:{rpc_server_port+i}",
f"tcp:{rpc_server_port+i}"]
- )
+
+ self._reverse_ports(rpc_tracker_port)
+ self._forward_ports(rpc_server_port, existing_forwards)
Review Comment:
Nit: Since `existing_forwards` isn't used elsewhere, may be cleaner to
generated it inside `_forward_ports`.
--
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]
