Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[rpc] Implemented rpc logging #11232

Merged
merged 3 commits into from
May 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,7 @@ list(APPEND COMPILER_SRCS "src/target/datatype/myfloat/myfloat.cc")
tvm_file_glob(GLOB RUNTIME_SRCS
src/runtime/*.cc
src/runtime/vm/*.cc
src/runtime/minrpc/*.cc
)

if(BUILD_FOR_HEXAGON)
Expand Down
12 changes: 12 additions & 0 deletions include/tvm/runtime/c_runtime_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,18 @@ extern "C" {
#endif
#include <stddef.h>
#include <stdint.h>
#include <stdio.h>
#include <sys/types.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mkatanbaf @mehrdadh @areusch why is this header needed ?
Is it just because to use the type ssize_t ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also do we want this added to c_runtime_api ?
(As this is affecting bare metal deployments)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, I forgot about that. I believe this was needed on Windows to define tvm_ssize_t properly. @mkatanbaf could you confirm that? If so, perhaps we can place this #include behind #ifdef _MSC_VER

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@manupa-arm @areusch The <stdio.h> is needed to define ssize_t (not on windows, for windows we directly used int64_t and int32_t instead of ssize_t). The <sys/type.h> was needed on hexagon.
We discussed the proper place to add this definition and landed on the c_runtime_api. @manupa-arm please lmk if you have another suggestion.

Copy link
Contributor

@manupak manupak May 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a naive look at the PR, It seems it is being used in rpc related sources (that uses PosixWrite and already assumes the presense of the OS). How about somewhere that is specific to rpc ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@manupa-arm thanks. there was an issue with ssize_t being defined in other parts of the code which caused a redefinition error. our plan was to unify the different definitions in a subsequent PR soon. I'll check if I can find a better place for the definition in order to minimize the effect on bare metal deployments.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps we can move this to e.g. tvm/support/types.h or something of that nature.


#if defined(_MSC_VER)
#if defined(_WIN64)
typedef int64_t tvm_ssize_t;
#else
typedef int32_t tvm_ssize_t;
#endif
#else
typedef ssize_t tvm_ssize_t;
#endif

/*! \brief type of array index. */
typedef int64_t tvm_index_t;
Expand Down
1 change: 1 addition & 0 deletions python/tvm/micro/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ def __enter__(self):
int(timeouts.session_start_timeout_sec * 1e6),
int(timeouts.session_established_timeout_sec * 1e6),
self._cleanup,
False,
)
)
self.device = self._rpc.cpu(0)
Expand Down
13 changes: 9 additions & 4 deletions python/tvm/rpc/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,9 @@ def request_and_run(self, key, func, priority=1, session_timeout=0, max_retry=2)
)


def connect(url, port, key="", session_timeout=0, session_constructor_args=None):
def connect(
url, port, key="", session_timeout=0, session_constructor_args=None, enable_logging=False
):
"""Connect to RPC Server

Parameters
Expand All @@ -483,6 +485,9 @@ def connect(url, port, key="", session_timeout=0, session_constructor_args=None)
The first element of the list is always a string specifying the name of
the session constructor, the following args are the positional args to that function.

enable_logging: boolean
flag to enable/disable logging. Logging is disabled by default.

Returns
-------
sess : RPCSession
Expand All @@ -503,9 +508,9 @@ def connect(url, port, key="", session_timeout=0, session_constructor_args=None)
.. code-block:: python

client_via_proxy = rpc.connect(
proxy_server_url, proxy_server_port, proxy_server_key,
proxy_server_url, proxy_server_port, proxy_server_key, enable_logging
session_constructor_args=[
"rpc.Connect", internal_url, internal_port, internal_key])
"rpc.Connect", internal_url, internal_port, internal_key, internal_logging])

"""
try:
Expand All @@ -514,7 +519,7 @@ def connect(url, port, key="", session_timeout=0, session_constructor_args=None)
session_constructor_args = session_constructor_args if session_constructor_args else []
if not isinstance(session_constructor_args, (list, tuple)):
raise TypeError("Expect the session constructor to be a list or tuple")
sess = _ffi_api.Connect(url, port, key, *session_constructor_args)
sess = _ffi_api.Connect(url, port, key, enable_logging, *session_constructor_args)
except NameError:
raise RuntimeError("Please compile with USE_RPC=1")
return RPCSession(sess)
Expand Down
2 changes: 0 additions & 2 deletions src/runtime/crt/microtvm_rpc_server/rpc_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,6 @@ class MicroRPCServer {
} // namespace runtime
} // namespace tvm

void* operator new[](size_t count, void* ptr) noexcept { return ptr; }

extern "C" {

static microtvm_rpc_server_t g_rpc_server = nullptr;
Expand Down
8 changes: 8 additions & 0 deletions src/runtime/micro/micro_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@

#include "../../support/str_escape.h"
#include "../rpc/rpc_channel.h"
#include "../rpc/rpc_channel_logger.h"
#include "../rpc/rpc_endpoint.h"
#include "../rpc/rpc_session.h"
#include "crt_config.h"
Expand Down Expand Up @@ -404,6 +405,13 @@ TVM_REGISTER_GLOBAL("micro._rpc_connect").set_body([](TVMArgs args, TVMRetValue*
throw std::runtime_error(ss.str());
}
std::unique_ptr<RPCChannel> channel(micro_channel);
bool enable_logging = false;
if (args.num_args > 7) {
enable_logging = args[7];
}
if (enable_logging) {
channel.reset(new RPCChannelLogging(std::move(channel)));
}
auto ep = RPCEndpoint::Create(std::move(channel), args[0], "", args[6]);
auto sess = CreateClientSession(ep);
*rv = CreateRPCSessionModule(sess);
Expand Down
93 changes: 93 additions & 0 deletions src/runtime/minrpc/minrpc_interfaces.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

#ifndef TVM_RUNTIME_MINRPC_MINRPC_INTERFACES_H_
#define TVM_RUNTIME_MINRPC_MINRPC_INTERFACES_H_

#include <tvm/runtime/c_runtime_api.h>

#include "rpc_reference.h"

namespace tvm {
namespace runtime {

/*!
* \brief Return interface used in ExecInterface to generate and send the responses.
*/
class MinRPCReturnInterface {
public:
virtual ~MinRPCReturnInterface() {}
/*! * \brief sends a response to the client with kTVMNullptr in payload. */
virtual void ReturnVoid() = 0;

/*! * \brief sends a response to the client with one kTVMOpaqueHandle in payload. */
virtual void ReturnHandle(void* handle) = 0;

/*! * \brief sends an exception response to the client with a kTVMStr in payload. */
virtual void ReturnException(const char* msg) = 0;

/*! * \brief sends a packed argument sequnce to the client. */
virtual void ReturnPackedSeq(const TVMValue* arg_values, const int* type_codes, int num_args) = 0;

/*! * \brief sends a copy of the requested remote data to the client. */
virtual void ReturnCopyFromRemote(uint8_t* data_ptr, uint64_t num_bytes) = 0;

/*! * \brief sends an exception response to the client with the last TVM erros as the message. */
virtual void ReturnLastTVMError() = 0;

/*! * \brief internal error. */
virtual void ThrowError(RPCServerStatus code, RPCCode info = RPCCode::kNone) = 0;
};

/*!
* \brief Execute interface used in MinRPCServer to process different received commands
*/
class MinRPCExecInterface {
public:
virtual ~MinRPCExecInterface() {}

/*! * \brief Execute an Initilize server command. */
virtual void InitServer(int num_args) = 0;

/*! * \brief calls a function specified by the call_handle. */
virtual void NormalCallFunc(uint64_t call_handle, TVMValue* values, int* tcodes,
int num_args) = 0;

/*! * \brief Execute a copy from remote command by sending the data described in arr to the client
*/
virtual void CopyFromRemote(DLTensor* arr, uint64_t num_bytes, uint8_t* data_ptr) = 0;

/*! * \brief Execute a copy to remote command by receiving the data described in arr from the
* client */
virtual int CopyToRemote(DLTensor* arr, uint64_t num_bytes, uint8_t* data_ptr) = 0;

/*! * \brief calls a system function specified by the code. */
virtual void SysCallFunc(RPCCode code, TVMValue* values, int* tcodes, int num_args) = 0;

/*! * \brief internal error. */
virtual void ThrowError(RPCServerStatus code, RPCCode info = RPCCode::kNone) = 0;

/*! * \brief return the ReturnInterface pointer that is used to generate and send the responses.
*/
virtual MinRPCReturnInterface* GetReturnInterface() = 0;
};

} // namespace runtime
} // namespace tvm
#endif // TVM_RUNTIME_MINRPC_MINRPC_INTERFACES_H_
Loading