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

WIP: Add cargo bloat action #167

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

ScottGibb
Copy link
Contributor

@ScottGibb ScottGibb commented Nov 14, 2024

PR Proposal: Tracking bloat on the library

This PR aims to add bloat tracking to our workflow. At present what im suggesting is a simple bot that will spit out the percentage of memory we using for each of our symbols (this can be tweaked to our preferences).

It will run on each pull request so that we can look at code size before we merge and post the sizings as comments on the PR. Each time the PR is updated the previous comment will be hidden(not deleted) so that we don't get too much noise on the actions.

Other things ive looked at

  • https://github.com/orf/cargo-bloat-action Originally my plan was to use this, but sadly it hasnt been updated in a while and thus does not support having crates in different folders. Instead I focused on a light weight method of just posting the output on the PR.

Testing

I would quite like to test it on this repo, but im unable to get it to work until the read write permissions are set. @lulf could you please enable this setting

This tool requires write permission, and that message means the requester does not have enough permission. Recently, GitHub sets permissions conservatively for newly created repositories. If it's a newly created repository, check your Settings > Actions > General > Workflow permissions, and make sure to enable read and write permissions.
https://github.com/marocchino/sticky-pull-request-comment/tree/v2/

@lulf
Copy link
Member

lulf commented Nov 14, 2024

So, code size for a given PR on it's own doesn't tell me that much, can it report the diff ocmpared to main ? Ideally this should also be runnable locally, i.e. part of ci.sh or some other script, so that devs can test before pushing the PR.

Another thing is that we should avoid spawning too many jobs as it will run into the max jobs of embassy-rs. I've not checked if bloat can be used with cargo batch (see ci.sh), but that might be an alternative to speed things up without the additional jobs.

@ScottGibb
Copy link
Contributor Author

I was originally going down the route of https://github.com/orf/cargo-bloat-action which gives a nice diff per function to see where things were changing. Sadly it doesnt seem to be maintained :(

can it report the diff ocmpared to main ?
Im looking into this at the moment, as i fiddle around with settings, triggers etc. Hoping to get something like that at the end

if bloat can be used with cargo batch (see ci.sh), but that might be an alternative to speed things up without the additional jobs.

Il have a look into this and get back to you :)

should also be runnable locally, i.e. part of ci.sh or some other script

I think this is the way we would have to go, in order to get a nice local diff, I think if we make our own script that is callable in ci.sh, this would give the benefits of both CI pipeline and developer experience.

@ScottGibb
Copy link
Contributor Author

Chatting with @petekubiak, potentially we could do a simplified script using https://github.com/rust-embedded/cargo-binutils to create a binary diff, thats callable in ci.sh for the developer and posts on the PR as well through the actionbot

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.

2 participants