[GitHub] [tvm] areusch commented on a diff in pull request #10967: [rpc] Implemented rpc logging

[GitBox <gi...@apache.org>]

areusch commented on code in PR #10967:
URL: https://github.com/apache/tvm/pull/10967#discussion_r855392123


##########
src/runtime/minrpc/minrpc_server.h:
##########
@@ -524,94 +523,249 @@ class MinRPCServer {
     int call_ecode = TVMSetStream(dev.device_type, dev.device_id, handle);
 
     if (call_ecode == 0) {
-      this->ReturnVoid();
+      ret_handler_->ReturnVoid();
     } else {
-      this->ReturnLastTVMError();
+      ret_handler_->ReturnLastTVMError();
     }
   }
 
   void ThrowError(RPCServerStatus code, RPCCode info = RPCCode::kNone) {
-    io_->Exit(static_cast<int>(code));
+    ret_handler_->ThrowError(code, info);
   }
 
+  ReturnInterface* GetReturnInterface() { return ret_handler_; }
+
+ private:
   template <typename T>
-  T* ArenaAlloc(int count) {
-    static_assert(std::is_pod<T>::value, "need to be trival");
-    return arena_.template allocate_<T>(count);
+  int ReadArray(T* data, size_t count) {
+    static_assert(std::is_trivial<T>::value && std::is_standard_layout<T>::value,
+                  "need to be trival");
+    return ReadRawBytes(data, sizeof(T) * count);
   }
 
-  template <typename T>
-  void Read(T* data) {
-    static_assert(std::is_pod<T>::value, "need to be trival");
-    this->ReadRawBytes(data, sizeof(T));
+  int ReadRawBytes(void* data, size_t size) {
+    uint8_t* buf = reinterpret_cast<uint8_t*>(data);
+    size_t ndone = 0;
+    while (ndone < size) {
+      ssize_t ret = io_->PosixRead(buf, size - ndone);
+      if (ret <= 0) return ret;
+      ndone += ret;
+      buf += ret;
+    }
+    return 1;
   }
 
-  template <typename T>
-  void ReadArray(T* data, size_t count) {
-    static_assert(std::is_pod<T>::value, "need to be trival");
-    return this->ReadRawBytes(data, sizeof(T) * count);
+  TIOHandler* io_;
+  ReturnInterface* ret_handler_;
+};
+
+/*!
+ * \brief A minimum RPC server that only depends on the tvm C runtime..
+ *
+ *  All the dependencies are provided by the io arguments.
+ *
+ * \tparam TIOHandler IO provider to provide io handling.
+ *         An IOHandler needs to provide the following functions:
+ *         - PosixWrite, PosixRead, Close: posix style, read, write, close API.
+ *         - MessageStart(num_bytes), MessageDone(): framing APIs.
+ *         - Exit: exit with status code.
+ */
+template <typename TIOHandler, template <typename> class Allocator = detail::PageAllocator>
+class MinRPCServer {
+ public:
+  using PageAllocator = Allocator<TIOHandler>;
+
+  /*!
+   * \brief Constructor.
+   * \param io The IO handler.
+   */
+  MinRPCServer(TIOHandler* io, ExecInterface* exec_handler)
+      : io_(io), arena_(PageAllocator(io_)), exec_handler_(exec_handler) {}
+
+  explicit MinRPCServer(TIOHandler* io)
+      : io_(io),
+        arena_(PageAllocator(io)),
+        ret_handler_(new MinRPCReturns<TIOHandler>(io_)),
+        exec_handler_(new MinRPCExecute<TIOHandler>(io_, ret_handler_)) {}

Review Comment:
   ah i see. i think we could likely remove operator new in rpc_server.cc if it's defined in <memory>. it's been long enough that I don't remember why i needed to explicitly override that anyhow--i do think though that we should verify by testing on Zephyr/Arduino to make sure <memory> behaves the same way there.



-- 
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: commits-unsubscribe@tvm.apache.org

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