You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2021/06/09 14:35:46 UTC

[GitHub] [tvm] fantasyRqg opened a new pull request #8223: support adb-shell style cpp_rpc

fantasyRqg opened a new pull request #8223:
URL: https://github.com/apache/tvm/pull/8223


   Fix problem mentioned at #7390
   
   Support `tvm_rpc` directly use in `adb shell` terminal
   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] FrozenGene commented on pull request #8223: support adb-shell style cpp_rpc

Posted by GitBox <gi...@apache.org>.
FrozenGene commented on pull request #8223:
URL: https://github.com/apache/tvm/pull/8223#issuecomment-864679700


   Thanks @fantasyRqg 


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] FrozenGene commented on pull request #8223: support adb-shell style cpp_rpc

Posted by GitBox <gi...@apache.org>.
FrozenGene commented on pull request #8223:
URL: https://github.com/apache/tvm/pull/8223#issuecomment-858210821


   please fix the format problem using clang-format-10


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] fantasyRqg commented on a change in pull request #8223: support adb-shell style cpp_rpc

Posted by GitBox <gi...@apache.org>.
fantasyRqg commented on a change in pull request #8223:
URL: https://github.com/apache/tvm/pull/8223#discussion_r648789399



##########
File path: .gitignore
##########
@@ -197,6 +197,7 @@ tvm_t.*
 .pytest_cache
 .local
 cmake-build-debug
+cmake-build*

Review comment:
       Should modify `.gitignore` in another PR ?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] FrozenGene commented on a change in pull request #8223: support adb-shell style cpp_rpc

Posted by GitBox <gi...@apache.org>.
FrozenGene commented on a change in pull request #8223:
URL: https://github.com/apache/tvm/pull/8223#discussion_r648788698



##########
File path: apps/cpp_rpc/rpc_env.cc
##########
@@ -95,7 +95,13 @@ RPCEnv::RPCEnv(const std::string& wd) {
     auto cmdline = fopen("/proc/self/cmdline", "r");
     fread(cwd, 1, sizeof(cwd), cmdline);
     fclose(cmdline);
-    base_ = "/data/data/" + std::string(cwd) + "/cache/rpc";
+    std::string android_base_ = "/data/data/" + std::string(cwd) +"/cache";
+    struct stat statbuf;
+    if (stat(android_base_.data(), &statbuf) == -1 || !S_ISDIR(statbuf.st_mode)){
+        android_base_ = ".";

Review comment:
       If failed, android_base_ becomes `.`,  will `./rpc` work well on android?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] fantasyRqg commented on a change in pull request #8223: support adb-shell style cpp_rpc

Posted by GitBox <gi...@apache.org>.
fantasyRqg commented on a change in pull request #8223:
URL: https://github.com/apache/tvm/pull/8223#discussion_r648789399



##########
File path: .gitignore
##########
@@ -197,6 +197,7 @@ tvm_t.*
 .pytest_cache
 .local
 cmake-build-debug
+cmake-build*

Review comment:
       Should I modify `.gitignore` in another PR ?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] FrozenGene merged pull request #8223: support adb-shell style cpp_rpc

Posted by GitBox <gi...@apache.org>.
FrozenGene merged pull request #8223:
URL: https://github.com/apache/tvm/pull/8223


   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] fantasyRqg commented on pull request #8223: support adb-shell style cpp_rpc

Posted by GitBox <gi...@apache.org>.
fantasyRqg commented on pull request #8223:
URL: https://github.com/apache/tvm/pull/8223#issuecomment-857752261


   Android Application test log:
   ```
   2021-06-09 22:21:48.114 12387-12428/com.rqg.tvm_rpc D/TVM_RUNTIME: /Users/rqg/Playground/tvm/apps/cpp_rpc/rpc_server.cc:129: bind to 0.0.0.0:9091
   2021-06-09 22:21:48.114 12387-12429/com.rqg.tvm_rpc D/TVM_RUNTIME: /Users/rqg/Playground/tvm/apps/cpp_rpc/rpc_tracker_client.h:194: Tracker connecting to 192.168.100.4:9091
   2021-06-09 22:21:58.230 12387-12429/com.rqg.tvm_rpc D/TVM_RUNTIME: /Users/rqg/Playground/tvm/apps/cpp_rpc/rpc_server.cc:295: Connection success 192.168.100.4:63009
   2021-06-09 22:21:58.532 12437-12437/? D/TVM_RUNTIME: /Users/rqg/Playground/tvm/apps/cpp_rpc/rpc_env.cc:126: Load module from /data/data/com.rqg.tvm_rpc/cache/rpc/cpu_lib.so ...
   2021-06-09 22:21:58.802 12437-12437/? D/TVM_RUNTIME: /Users/rqg/Playground/tvm/apps/cpp_rpc/rpc_server.cc:317: Finish serving 192.168.100.4:63009
   2021-06-09 22:21:58.841 12387-12429/com.rqg.tvm_rpc D/TVM_RUNTIME: /Users/rqg/Playground/tvm/apps/cpp_rpc/rpc_server.cc:200: Child pid=12436 killed, Process status = 15
   2021-06-09 22:21:58.841 12387-12429/com.rqg.tvm_rpc D/TVM_RUNTIME: /Users/rqg/Playground/tvm/apps/cpp_rpc/rpc_server.cc:224: Socket Connection Closed
   ```
   
   
   adb shell test log:
   ```
   /tvm_rpc server --port=9090 --tracker=192.168.100.4:9091 --key=android                                                                    <
   [22:23:41] /Users/rqg/Playground/tvm/apps/cpp_rpc/main.cc:260: ./tvm_rpc
   [22:23:41] /Users/rqg/Playground/tvm/apps/cpp_rpc/main.cc:260: server
   [22:23:41] /Users/rqg/Playground/tvm/apps/cpp_rpc/main.cc:260: --port=9090
   [22:23:41] /Users/rqg/Playground/tvm/apps/cpp_rpc/main.cc:260: --tracker=192.168.100.4:9091
   [22:23:41] /Users/rqg/Playground/tvm/apps/cpp_rpc/main.cc:260: --key=android
   [22:23:41] /Users/rqg/Playground/tvm/apps/cpp_rpc/main.cc:96: host        = 0.0.0.0
   [22:23:41] /Users/rqg/Playground/tvm/apps/cpp_rpc/main.cc:97: port        = 9090
   [22:23:41] /Users/rqg/Playground/tvm/apps/cpp_rpc/main.cc:98: port_end    = 9099
   [22:23:41] /Users/rqg/Playground/tvm/apps/cpp_rpc/main.cc:99: tracker     = ('192.168.100.4', 9091)
   [22:23:41] /Users/rqg/Playground/tvm/apps/cpp_rpc/main.cc:100: key         = android
   [22:23:41] /Users/rqg/Playground/tvm/apps/cpp_rpc/main.cc:101: custom_addr =
   [22:23:41] /Users/rqg/Playground/tvm/apps/cpp_rpc/main.cc:102: work_dir    =
   [22:23:41] /Users/rqg/Playground/tvm/apps/cpp_rpc/main.cc:103: silent      = False
   [22:23:41] /Users/rqg/Playground/tvm/apps/cpp_rpc/main.cc:266: Starting CPP Server, Press Ctrl+C to stop.
   [22:23:41] /Users/rqg/Playground/tvm/apps/cpp_rpc/main.cc:285: RPCServerCreate: ('192.168.100.4', 9091)
   [22:23:41] /Users/rqg/Playground/tvm/apps/cpp_rpc/rpc_server.cc:129: bind to 0.0.0.0:9090
   [22:23:41] /Users/rqg/Playground/tvm/apps/cpp_rpc/rpc_tracker_client.h:194: Tracker connecting to 192.168.100.4:9091
   [22:23:48] /Users/rqg/Playground/tvm/apps/cpp_rpc/rpc_server.cc:295: Connection success 192.168.100.4:63333
   [22:23:48] /Users/rqg/Playground/tvm/apps/cpp_rpc/rpc_env.cc:126: Load module from ./rpc/cpu_lib.so ...
   [22:23:48] /Users/rqg/Playground/tvm/apps/cpp_rpc/rpc_server.cc:317: Finish serving 192.168.100.4:63333
   [22:23:48] /Users/rqg/Playground/tvm/apps/cpp_rpc/rpc_server.cc:200: Child pid=12526 killed, Process status = 15
   [22:23:48] /Users/rqg/Playground/tvm/apps/cpp_rpc/rpc_server.cc:224: Socket Connection Closed
   ```


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] FrozenGene commented on a change in pull request #8223: support adb-shell style cpp_rpc

Posted by GitBox <gi...@apache.org>.
FrozenGene commented on a change in pull request #8223:
URL: https://github.com/apache/tvm/pull/8223#discussion_r651470581



##########
File path: apps/cpp_rpc/rpc_env.cc
##########
@@ -95,7 +96,16 @@ RPCEnv::RPCEnv(const std::string& wd) {
     auto cmdline = fopen("/proc/self/cmdline", "r");
     fread(cwd, 1, sizeof(cwd), cmdline);
     fclose(cmdline);
-    base_ = "/data/data/" + std::string(cwd) + "/cache/rpc";
+    std::string android_base_ = "/data/data/" + std::string(cwd) + "/cache";
+    struct stat statbuf;
+    // Check if application data directory exist. If not exist usually mean tvm_rpc run from adb

Review comment:
       Nitty comment. `If not exist, usually means we run tvm_rpc from adb shell terminal`

##########
File path: apps/cpp_rpc/rpc_env.cc
##########
@@ -95,7 +96,16 @@ RPCEnv::RPCEnv(const std::string& wd) {
     auto cmdline = fopen("/proc/self/cmdline", "r");
     fread(cwd, 1, sizeof(cwd), cmdline);
     fclose(cmdline);
-    base_ = "/data/data/" + std::string(cwd) + "/cache/rpc";
+    std::string android_base_ = "/data/data/" + std::string(cwd) + "/cache";
+    struct stat statbuf;
+    // Check if application data directory exist. If not exist usually mean tvm_rpc run from adb
+    // shell terminal.
+    if (stat(android_base_.data(), &statbuf) == -1 || !S_ISDIR(statbuf.st_mode)) {
+      // Tmp directory always writable for 'shell' user.

Review comment:
       `is always writable...`




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] FrozenGene commented on a change in pull request #8223: support adb-shell style cpp_rpc

Posted by GitBox <gi...@apache.org>.
FrozenGene commented on a change in pull request #8223:
URL: https://github.com/apache/tvm/pull/8223#discussion_r648798469



##########
File path: .gitignore
##########
@@ -197,6 +197,7 @@ tvm_t.*
 .pytest_cache
 .local
 cmake-build-debug
+cmake-build*

Review comment:
       Yes, if you think it is a must




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] FrozenGene commented on a change in pull request #8223: support adb-shell style cpp_rpc

Posted by GitBox <gi...@apache.org>.
FrozenGene commented on a change in pull request #8223:
URL: https://github.com/apache/tvm/pull/8223#discussion_r648786905



##########
File path: .gitignore
##########
@@ -197,6 +197,7 @@ tvm_t.*
 .pytest_cache
 .local
 cmake-build-debug
+cmake-build*

Review comment:
       your pr should not be related with this .gitignore




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] fantasyRqg commented on pull request #8223: support adb-shell style cpp_rpc

Posted by GitBox <gi...@apache.org>.
fantasyRqg commented on pull request #8223:
URL: https://github.com/apache/tvm/pull/8223#issuecomment-861126514


   @FrozenGene All problems solved


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] fantasyRqg commented on pull request #8223: support adb-shell style cpp_rpc

Posted by GitBox <gi...@apache.org>.
fantasyRqg commented on pull request #8223:
URL: https://github.com/apache/tvm/pull/8223#issuecomment-863897060


   @FrozenGene Thanks for correcting my spelling mistake [Lol]


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] fantasyRqg commented on a change in pull request #8223: support adb-shell style cpp_rpc

Posted by GitBox <gi...@apache.org>.
fantasyRqg commented on a change in pull request #8223:
URL: https://github.com/apache/tvm/pull/8223#discussion_r648791852



##########
File path: apps/cpp_rpc/rpc_env.cc
##########
@@ -95,7 +95,13 @@ RPCEnv::RPCEnv(const std::string& wd) {
     auto cmdline = fopen("/proc/self/cmdline", "r");
     fread(cwd, 1, sizeof(cwd), cmdline);
     fclose(cmdline);
-    base_ = "/data/data/" + std::string(cwd) + "/cache/rpc";
+    std::string android_base_ = "/data/data/" + std::string(cwd) +"/cache";
+    struct stat statbuf;
+    if (stat(android_base_.data(), &statbuf) == -1 || !S_ISDIR(statbuf.st_mode)){
+        android_base_ = ".";

Review comment:
       When current working directory is writable, Yes .
   
   Another option is force `base_` to `/data/local/tmp/rpc`. But I don't think this is a good choice. So I keep the original implementation .
   




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] FrozenGene commented on a change in pull request #8223: support adb-shell style cpp_rpc

Posted by GitBox <gi...@apache.org>.
FrozenGene commented on a change in pull request #8223:
URL: https://github.com/apache/tvm/pull/8223#discussion_r648799216



##########
File path: apps/cpp_rpc/rpc_env.cc
##########
@@ -95,7 +95,13 @@ RPCEnv::RPCEnv(const std::string& wd) {
     auto cmdline = fopen("/proc/self/cmdline", "r");
     fread(cwd, 1, sizeof(cwd), cmdline);
     fclose(cmdline);
-    base_ = "/data/data/" + std::string(cwd) + "/cache/rpc";
+    std::string android_base_ = "/data/data/" + std::string(cwd) +"/cache";
+    struct stat statbuf;
+    if (stat(android_base_.data(), &statbuf) == -1 || !S_ISDIR(statbuf.st_mode)){
+        android_base_ = ".";

Review comment:
       how about add one simple code comment to indicate users if failed, should make sure the directory is writable and also consider to change `base_` to `/data/local/tmp/rpc`. This could help users find one solution more quickly




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org