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] Added native debug logging to Android RPC #1432

Merged
merged 5 commits into from
Jul 18, 2018
Merged

[RPC] Added native debug logging to Android RPC #1432

merged 5 commits into from
Jul 18, 2018

Conversation

alex-weaver
Copy link
Contributor

Native code on Android does not automatically forward stdout/stderr to logcat. This PR sets up custom logging so that log messages from TVM are visible in the logcat output. This makes debugging issues in native TVM significantly easier.

@alex-weaver alex-weaver changed the title [RPC] Added native debug printing to Android RPC [RPC] Added native debug logging to Android RPC Jul 14, 2018
@tqchen
Copy link
Member

tqchen commented Jul 15, 2018

cc @eqy can you review this?

#include <android/log.h>

void dmlc::CustomLogMessage::Log(const std::string& msg) {
__android_log_print(ANDROID_LOG_DEBUG, "TVM_RUNTIME", "%s", msg.c_str());
Copy link
Contributor

Choose a reason for hiding this comment

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

i guess __android_log_print(ANDROID_LOG_DEBUG, "TVM_RUNTIME", msg.c_str()); can give us what we want without the "%s", ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Could simplify like

#define LOGD(MSG) __android_log_print(ANDROID_LOG_DEBUG , "TVM_RUNTIME",MSG);

LOGD("Welcome")

Copy link
Contributor Author

@alex-weaver alex-weaver Jul 15, 2018

Choose a reason for hiding this comment

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

@jackwish __android_log_print assumes the string is a printf-style format string, so just passing in msg wouldn't work if the log message contained any formatting directives. However, there is an __android_log_write which does do this. I'll get this changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@srkreddy1238 I don't think that preserves the functionality of this PR. The intent is to capture all log messages that TVM generates, and pass them on to logcat. Without this, there is no way to observe the log messages generated by the native TVM code in android app (we would otherwise only see messages from java code).

@@ -6,6 +6,9 @@
#include <sys/stat.h>
#include <fstream>

#define DMLC_LOG_CUSTOMIZE 1
#define DMLC_LOG_BEFORE_THROW 1
Copy link
Contributor

Choose a reason for hiding this comment

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

purpose of these macros ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DMLC_LOG_CUSTOMIZE is what tells TVM to use the CustomMessage class for logging instead of LogMessage. This requires us to define an implementation for the static function dmlc::CustomLogMessage::Log, which will be invoked for every log that TVM records. This is what allows us to pass the log messages to one of the android logging functions so that they will show up in logcat.

DMLC_LOG_BEFORE_THROW ensures that the log function is invoked before throwing in LogMessageFatal, so that again we have an opportunity to pass the message on to logcat.

see dmlc-core/include/dmlc/logging.h for the implementation of both of these

#include <android/log.h>

void dmlc::CustomLogMessage::Log(const std::string& msg) {
__android_log_print(ANDROID_LOG_DEBUG, "TVM_RUNTIME", "%s", msg.c_str());
Copy link
Contributor

Choose a reason for hiding this comment

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

Could simplify like

#define LOGD(MSG) __android_log_print(ANDROID_LOG_DEBUG , "TVM_RUNTIME",MSG);

LOGD("Welcome")

@tqchen
Copy link
Member

tqchen commented Jul 17, 2018

@srkreddy1238 makes a good comment on clarifying the macros, @alex-weaver can you add two lines of comments? @jackwish please https://docs.tvm.ai/contribute/code_review.html#approve-and-request-changes-explicitly

@tqchen tqchen added status: review in progress status: need update need update based on feedbacks labels Jul 17, 2018
Copy link
Contributor

@srkreddy1238 srkreddy1238 left a comment

Choose a reason for hiding this comment

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

LGTM

@tqchen tqchen merged commit b6418b9 into apache:master Jul 18, 2018
@tqchen
Copy link
Member

tqchen commented Jul 18, 2018

THanks @alex-weaver @srkreddy1238 @jackwish This is merged

tqchen pushed a commit to tqchen/tvm that referenced this pull request Aug 4, 2018
sergei-mironov pushed a commit to sergei-mironov/tvm that referenced this pull request Aug 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants