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

Add colors and user-configurable backtrace limit #9872

Closed
wants to merge 2 commits into from

Conversation

driazati
Copy link
Member

@driazati driazati commented Jan 7, 2022

This adds colors (which match GDB's output) to files / symbols when printing exceptions from C++ if stdout is a terminal. This also adds an environment variable TVM_BACKTRACE_LIMIT which can be used to limit the length of backtraces at runtime. The combo of these two helps make backtraces easier to read at a glance.

export TVM_BACKTRACE_LIMIT=5
python some_code_with_an_error.py

image

Piping through something disables colors

python some_code_with_an_error.py | cat

image

cc @tkonolige

This adds colors (which match GDB's output) to files / symbols when printing exceptions from C++ if stdout is a terminal. This also adds an environment variable `TVM_BACKTRACE_LIMIT` which can be used to limit the length of backtraces at runtime. The combo of these two helps make backtraces easier to read at a glance.

```bash
export TVM_BACKTRACE_LIMIT=5
python some_code_with_an_error.py
```
@junrushao
Copy link
Member

adding colors is certainly super color feature! BTW, is it possible not to introduce platform dependent code? Note that we have rang as the 3rdparty repo

@tkonolige
Copy link
Contributor

The colors look great! How does this interact with backtraces in python? Are only the c++ parts of the backtrace colored?

@junrushao
Copy link
Member

BTW, there is an pre-RFC discussing logging: https://discuss.tvm.apache.org/t/rfc-better-tvm-logger-on-both-c-and-python-side/11734/13. Let's post this PR to the thread for more visibility :-)

@driazati
Copy link
Member Author

driazati commented Jan 7, 2022

The colors look great! How does this interact with backtraces in python? Are only the c++ parts of the backtrace colored?

It doesn't touch Python at all since those are (I assume) still coming from Python itself, just the bespoke stack backtrace string logging.cc creates that gets handed up from C++.

@tkonolige
Copy link
Contributor

So things are a little more complicated here than it appears at first glance. And error thrown by tvm via the LOG and CHECK infrastructure constructs a TVMError with a backtrace. When this error hits the python/c++ language boundary it is translated into a new python error (the entries in the stacktrace get reordered). However, there is no guarantee that this error will be output to stdout. That means in some cases the current approach you have will not work. I think a better solution is to have the python exception detect if is is being written to a terminal and color the output then. But I don't know if there is a way to do this. Also, making the stacktrace structured might facilitate doing the coloring after the fact.

@driazati
Copy link
Member Author

Right, seems like we'd need to wait on something like this for if/when we implement more structured exceptions and can avoid any tricky parsing. This could still go in hidden behind some env variable so developers have to explicitly turn it on, maybe something like TVM_BACKTRACE_COLOR=always though

@tkonolige
Copy link
Contributor

I don't know if this is my decision to make, but I see two ways forward:

  1. Delay this PR until we have structured errors. This will probably take a while because there isn't anyone working on structured errors right now.
  2. Hide this feature behind a TVM_BACKTRACE_COLOR flag.

Personally I think 1 is the right way to do this but 2 might be a good stopgap. However, if we do 2 we first need to check that coloring errors does not cause issues with any of the tuning infrastructure. I know autotvm saves the errors to a log file, so you'll have to test that the colors get disabled for that.

Comment on lines +134 to +148
// Limit backtrace length based on TVM_BACKTRACE_LIMIT env variable
auto user_limit_s = getenv("TVM_BACKTRACE_LIMIT");
const auto default_limit = 500;

if (user_limit_s == nullptr) {
bt.max_size = default_limit;
} else {
// Parse out the user-set backtrace limit
try {
bt.max_size = std::stoi(user_limit_s);
} catch (const std::invalid_argument& e) {
bt.max_size = default_limit;
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

This should be a separate PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

moved into #10025

@driazati driazati marked this pull request as draft January 19, 2022 21:08
@driazati
Copy link
Member Author

Closing for now, will revisit when/if structured exceptions are a thing

@driazati driazati closed this Jan 22, 2022
driazati added a commit to driazati/tvm that referenced this pull request Jan 22, 2022
A spin off of apache#9872, this adds an env variable `TVM_BACKTRACE_LIMIT` which can be set to an integer to limit the frames printed out on errors. This can make it easier to run interactive TVM scripts with errors since the stack traces are often long (70+ frames).

```bash
export TVM_BACKTRACE_LIMIT=5
python some_code_with_an_error.py
```

cc @tkonolige
junrushao pushed a commit that referenced this pull request Jan 22, 2022
A spin off of #9872, this adds an env variable `TVM_BACKTRACE_LIMIT` which can be set to an integer to limit the frames printed out on errors. This can make it easier to run interactive TVM scripts with errors since the stack traces are often long (70+ frames).

```bash
export TVM_BACKTRACE_LIMIT=5
python some_code_with_an_error.py
```

cc @tkonolige

Co-authored-by: driazati <driazati@users.noreply.github.com>
yuanfz98 pushed a commit to yuanfz98/tvm that referenced this pull request Jan 24, 2022
A spin off of apache#9872, this adds an env variable `TVM_BACKTRACE_LIMIT` which can be set to an integer to limit the frames printed out on errors. This can make it easier to run interactive TVM scripts with errors since the stack traces are often long (70+ frames).

```bash
export TVM_BACKTRACE_LIMIT=5
python some_code_with_an_error.py
```

cc @tkonolige

Co-authored-by: driazati <driazati@users.noreply.github.com>
ylc pushed a commit to ylc/tvm that referenced this pull request Feb 16, 2022
A spin off of apache#9872, this adds an env variable `TVM_BACKTRACE_LIMIT` which can be set to an integer to limit the frames printed out on errors. This can make it easier to run interactive TVM scripts with errors since the stack traces are often long (70+ frames).

```bash
export TVM_BACKTRACE_LIMIT=5
python some_code_with_an_error.py
```

cc @tkonolige

Co-authored-by: driazati <driazati@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants