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

Allow use of llvm-addr2line as the command #18

Merged
merged 1 commit into from
Apr 7, 2022

Conversation

travisdowns
Copy link
Member

Prior to this change the addr2line.py script always uses addr2line as the
binary to decode stack frames. This change allows the path and/or name
of the binary to be provided explicitly on the command line.

This allows the of llvm-addr2line, an alternative implementation
based on llvm-symbolizer, which in my experiments is over 200 times as
fast as addr2line in decoding some redpanda stack traces.

See https://sourceware.org/bugzilla/show_bug.cgi?id=29010 for more
on the addr2line slowness.

This change also slightly changes the 'dummy line' strategy used to
detect when addr2line has finished outputting frames for the current
address to make it compatible with llvm-addr2line.

Fixes scylladb#1035.

Prior to this change the addr2line.py script always uses addr2line as the
binary to decode stack frames. This change allows the path and/or name
of the binary to be provided explicitly on the command line.

This allows the of llvm-addr2line, an alternative implementation
based on llvm-symbolizer, which in my experiments is over *200* times as
fast as addr2line in decoding some redpanda stack traces.

See https://sourceware.org/bugzilla/show_bug.cgi?id=29010 for more
on the addr2line slowness.

This change also slightly changes the 'dummy line' strategy used to
detect when addr2line has finished outputting frames for the current
address to make it compatible with llvm-addr2line.

Fixes scylladb#1035.
@CLAassistant
Copy link

CLAassistant commented Mar 31, 2022

CLA assistant check
All committers have signed the CLA.

@travisdowns travisdowns requested review from dotnwat and jcsp March 31, 2022 20:38
r"(.*0x0000000000000000: \?\? \?\?:0\n)" # addr2line pattern
r"|"
r"(.*0x0: \?\? at \?\?:0\n)" # llvm-addr2line pattern
)
Copy link
Member Author

Choose a reason for hiding this comment

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

The main complication is the slightly differnet "end of backtrace" beahvior for llvm-addr2line: the 0x0 frame we use to find it looks a bit different. Use a regex to accommodate the differences.

Copy link
Member

@dotnwat dotnwat left a comment

Choose a reason for hiding this comment

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

lgtm.

after this merges then the next step is to update the sha1 in vtool's 3rdparty.cmake so that this will get pulled down into everyone's tree?

@psarna
Copy link

psarna commented Apr 1, 2022

Hello Seastar cousins, thanks for discovering the issue and feel free to post a pull request upstream as well :) Pull requests are also accepted in scylladb/seastar, so no need for a separate mailing list patch.

@dotnwat
Copy link
Member

dotnwat commented Apr 1, 2022

Hello Seastar cousins ... are also accepted in scylladb/seastar, so no need for a separate mailing list patch.

:) That's wonderful news @psarna

@travisdowns
Copy link
Member Author

after this merges then the next step is to update the sha1 in vtool's 3rdparty.cmake so that this will get pulled down into everyone's tree?

Yes! I'll do that next.

@dotnwat
Copy link
Member

dotnwat commented Apr 6, 2022

@jcsp everything look good here to you?

Copy link

@jcsp jcsp left a comment

Choose a reason for hiding this comment

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

LGTM

@travisdowns travisdowns merged commit 2a9504b into redpanda-data:master Apr 7, 2022
@travisdowns travisdowns deleted the issue-1035 branch April 7, 2022 20:50
@travisdowns travisdowns restored the issue-1035 branch April 11, 2022 18:23
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.

seastar-addr2line can take minutes to decode a backtrace
5 participants