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

Backtrace performance #283

Merged
merged 1 commit into from
Aug 5, 2017
Merged

Backtrace performance #283

merged 1 commit into from
Aug 5, 2017

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Aug 4, 2017

Only collect backtrace when RUST_BACKTRACE is non-empty. Also, resolve symbols lazily when printing.

@oli-obk
Copy link
Contributor

oli-obk commented Aug 4, 2017

rust-lang/backtrace-rs@3d96a92 allows deferring the work now. Would need a git dependency though.

@RalfJung
Copy link
Member Author

RalfJung commented Aug 4, 2017

Done.

} else {
write!(trace_text, "\n").unwrap();
for symbol in frame.symbols() {
write!(trace_text, "{}: " , i).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

the stack frame number is now reported for every symbol, instead of once for all symbols at that frame. Is that an intendend change?

Copy link
Member Author

@RalfJung RalfJung Aug 4, 2017

Choose a reason for hiding this comment

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

Yes. This brings the trace down to two lines per frame for the common case (one symbol per frame), making it more compact and easier to read. It also looks more like backtraces printed on panics.

@oli-obk oli-obk merged commit b59c092 into rust-lang:master Aug 5, 2017
@RalfJung RalfJung deleted the backtrace branch August 29, 2017 14:13
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.

2 participants