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 flamegraph of a Tx #29

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Add flamegraph of a Tx #29

wants to merge 9 commits into from

Conversation

FrancoGiachetta
Copy link
Contributor

Refers to #27
This PR adds a flamegraph of a random tx, so as to know how much time is spent in each part of the process of its execution.

@FrancoGiachetta FrancoGiachetta changed the title add flamegraph + update readme Add flamegraph of a Tx Jul 8, 2024
@FrancoGiachetta FrancoGiachetta marked this pull request as ready for review July 8, 2024 17:50
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should not add the flamegraph to the repo!

README.md Outdated
cargo install flamegraph
```

#### Example
Copy link
Collaborator

Choose a reason for hiding this comment

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

Lets put something like

To produce a flamegraph of a transaction execution run:

@FrancoGiachetta
Copy link
Contributor Author

Changes were done

@JulianGCalderon
Copy link
Contributor

The flamegraph looks a bit odd to me. I ran the command and saw that most of the samples are from Clap (argument parsing) and Tracing (logging), while blockifier, cairo_native and blockifier_state_reader (the crates that take most of the execution time) are not shown. It seems that, altogether, external crates are not being sampled. Is there a way to include those crates in the flamegraph?

flamegraph

@FrancoGiachetta
Copy link
Contributor Author

mmm, I see. To be honest, it looked strange to me, especially because it finishes quite fast. I guess I should take deeper look into it

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.

4 participants