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.

Sign up for GitHub

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

Jump to bottom

linux: fixes perf profiling stacktraces #244

Merged
merged 9 commits into from
Dec 30, 2021
Merged

linux: fixes perf profiling stacktraces #244

merged 9 commits into from
Dec 30, 2021

Conversation

RafaelGSS
Copy link
Contributor

@RafaelGSS RafaelGSS commented Dec 28, 2021 •
edited
Loading

Address: #242

This PR also adds support to categorization through perf stack traces.

@RafaelGSS
Copy link
Contributor Author

Quick feedback. I'm still working in the PR because it's missing:

  • A better parser of functions with categories (CPP, V8, JS)

image

  • Inlined functions

@RafaelGSS RafaelGSS mentioned this pull request Dec 29, 2021
Merged
@RafaelGSS RafaelGSS changed the title fix: kill perf script on sigint linux: fixes perf profiling stacktraces Dec 29, 2021
@RafaelGSS RafaelGSS marked this pull request as ready for review December 29, 2021 21:53
@RafaelGSS
Copy link
Contributor Author

RafaelGSS commented Dec 29, 2021 •
edited
Loading

While I look into how to test those changes, this PR is ready to review.

Also, is missing the inlined functions, I'm looking into V8 options to see how inlined functions act when --perf-basic-prof is passed. Otherwise, I'm thinking in to use --print-opt-source along with perf traces to track it down.

@RafaelGSS
Copy link
Contributor Author

The 0x now creates a flamegraph like this one:

image

(It was not even generated in the master version)

Also, notice that ^etag.js and etag.js are considered different stack-line, someone knows why?

mcollina
mcollina approved these changes Dec 30, 2021
View reviewed changes
Copy link
Collaborator

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina merged commit 188a67b into davidmarkclements:master Dec 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcollina mcollina mcollina approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@RafaelGSS @mcollina