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

unwinder/native: Per code region unwinder selection #2419

Merged
merged 1 commit into from Jan 11, 2024

Conversation

javierhonduco
Copy link
Contributor

@javierhonduco javierhonduco commented Jan 10, 2024

Before this commit we either used frame pointers or DWARF-derived unwind
information to unwind a process. We first attempt to unwind using frame
pointers and if it looks like it worked (we can't be sure about this,
see comments for more details) we aggregate the sample, otherwise we ask
userspace to write unwind information, if present. We have to exceptions
for this case, we never generated unwind info for v8-based apps 0 nor
for Go-based apps [1].

This commit enables per code region switching of unwinders, what this
means in practice is that it will allow us to unwind Go or V8
applications, that should have frame pointers in the majority of their
code using the FP unwinder, but switch to the DWARF-derived unwinder for
part of the code that do not have them, such as for native libraries
written in C or C++ that were compiled without them. Our agent is a good
example of this as we rely on CGO with a bunch of libraries that can be
compiled without FPs.

Additionally, this work also adds support for unwinding interpreters
whose native code sections contain FPs.

  • [1]: Go has FP enabled by default

Test Plan

Tested on the Agent, CI is clean,

Signed-off-by: Francisco Javier Honduvilla Coto javierhonduco@gmail.com

@javierhonduco javierhonduco changed the title wip: fp <-> dwarf switching unwinder/native: Per code region unwinder selection Jan 11, 2024
@javierhonduco javierhonduco marked this pull request as ready for review January 11, 2024 13:22
@javierhonduco javierhonduco requested a review from a team as a code owner January 11, 2024 13:22
Before this commit we either used frame pointers or DWARF-derived unwind
information to unwind a process. We first attempt to unwind using frame
pointers and if it looks like it worked (we can't be sure about this,
see comments for more details) we aggregate the sample, otherwise we ask
userspace to write unwind information, if present. We have to exceptions
for this case, we never generated unwind info for v8-based apps [0] nor
for Go-based apps [1].

This commit enables per code region switching of unwinders, what this
means in practice is that it will allow us to unwind Go or V8
applications, that should have frame pointers in the majority of their
code using the FP unwinder, but switch to the DWARF-derived unwinder for
part of the code that do not have them, such as for native libraries
written in C or C++ that were compiled without them. Our agent is a good
example of this as we rely on CGO with a bunch of libraries that can be
compiled without FPs.

Additionally, this work also adds support for unwinding interpreters
whose native code sections contain FPs.

- [0]: https://www.polarsignals.com/blog/posts/2023/07/12/correctly-profiling-node-js-with-zero-instrumentation
- [1]: Go has FP enabled by default

Test Plan
=========

Tested on the Agent, CI is clean,

Signed-off-by: Francisco Javier Honduvilla Coto <javierhonduco@gmail.com>
@Sylfrena Sylfrena merged commit 02bbef2 into main Jan 11, 2024
25 checks passed
@javierhonduco javierhonduco deleted the wip-native-improvements branch January 12, 2024 13:16
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

2 participants