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

Comment output of cachegrind in PR #797

Closed
rakita opened this issue Oct 12, 2023 · 6 comments
Closed

Comment output of cachegrind in PR #797

rakita opened this issue Oct 12, 2023 · 6 comments
Labels
ci CI related

Comments

@rakita
Copy link
Member

rakita commented Oct 12, 2023

Not sure how to do this but would be very beneficial

As i am on mac valgrind does not work and i have a Docker file:

FROM alpine:latest
RUN apk add g++ gcc clang-dev valgrind curl git llvm

# Install rust
RUN curl https://sh.rustup.rs -sSf | sh -s -- -y

# Add .cargo/bin to PATH
ENV PATH="/root/.cargo/bin:${PATH}"

# Check cargo is visible
RUN cargo --help

Run docker with:
docker run -it -v $PWD:/tmp -w /tmp valgrind:1.0

And I run snailtracer to get results like this:

cargo b -r -p revm-test --bin snailtracer
valgrind --tool=cachegrind target/release/snailtracer
@rakita rakita added the ci CI related label Dec 25, 2023
@0x0elliot
Copy link
Contributor

Can i help out with this issue? Seems simple enough as an entrypoint for someone who's new to the EVM ecosystem.

@rakita
Copy link
Member Author

rakita commented Dec 26, 2023

Can i help out with this issue? Seems simple enough as an entrypoint for someone who's new to the EVM ecosystem.

Go for it, it is mostly github actions and how to output it in PR (not sure what github plugging are out there for it)

@0x0elliot
Copy link
Contributor

@rakita A bit confused as to what needs to be done. Do you mind giving me a bit of context? I reassure you, this would get lesser and lesser with the next few contributions.

If you wish, I can ping you somewhere else too (a discord server, the revm telegram group etc)

@rakita
Copy link
Member Author

rakita commented Dec 26, 2023

@rakita A bit confused as to what needs to be done. Do you mind giving me a bit of context? I reassure you, this would get lesser and lesser with the next few contributions.

If you wish, I can ping you somewhere else too (a discord server, the revm telegram group etc)

Idea is to run valgrind cachegrind on every new PR.

cachegrind is used to deduce the number of executed instructions and is a good metric to check how fast/slow the program is. In comparison, time measurement can be variable and hard to get one value, while num of the instruction is always the same for the same program so it is a good ballpark.

So executing this on every PR means adding github action: https://github.com/bluealloy/revm/tree/main/.github/workflows and using some (i hope it exists) github plugin that outputs this to the PR as a comment.

Hope that this makes it clearer.

@0x0elliot
Copy link
Contributor

0x0elliot commented Dec 26, 2023

Thanks! That does. Let me play around with this and come up with a draft.

rakita pushed a commit that referenced this issue Dec 30, 2023
* ci: adding cachegrind to PRs!

* ci: making it trigger everytime a PR opens and a comment is made

* ci: simplifying run rules

* ci: making it work

* ci: triggering again to see

* ci: fixing arguments

* ci: installing valgrind

* ci: installing cg_annotate

* ci: playing around a bit more

* ci: this should get the first working comment

* ci: this should get the first working comment (1)

* ci: this should get the first working comment (2 :P)

* ci: this should get the first working comment (3 :P)

* ci: trying again lol

* ci: trying again lol

* ci: trying again lol

* ci: trying again lol

* ci: trying again lol

* ci: trying again lol

* ci: debugging

* ci: debugging

* ci: debugging

* ci: debugging

* ci: debugging

* ci: debugging

* ci: debugging

* ci: debugging

* ci: debugging

* ci: debugging

* ci: debugging

* ci: debugging

* ci: debugging

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: sigh

* ci: removing garbage

* ci: adding edit last flag

* ci: testing around with no previous comment edge case

* ci: testing with permissions

* ci: giving workflow further more permissions

* ci: testing workflow
@rakita
Copy link
Member Author

rakita commented Jan 9, 2024

Still need to fix PAT and github token but issue is considered done

@rakita rakita closed this as completed Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci CI related
Projects
None yet
Development

No branches or pull requests

2 participants