Add github action to report memory diff user diffsyms tool #6962
Conversation
| if: ${{ github.event_name == 'pull_request' }} | ||
| with: | ||
| workflow: examples-esp32.yaml | ||
| name: esp32-example-build-${{ github.event.pull_request.base.ref }} |
There was a problem hiding this comment.
What will be the effect if the base binaries are not available (we are frequently running out of quota so have quite aggressive cleanup scripts)?
Trying to ensure that our CI does not turn red on this.
There was a problem hiding this comment.
As our master branch is moving forward, new base binaries will be build for every commit. One can rebase his PR on the newest master to get a new image.
This part is not essential, so even it fails, it doesn't cause any trouble.
And I have several other ideas to reduce the usage of artifacts. Let me investigate them later
There was a problem hiding this comment.
I guess my comment would be translated to:
Do you need a continue-on-error: true here?
|
Please fill up the 'Summary' for this PR. Especially important: how did you test? The Github script should be manually testable at least in some PR where we could see the expected output. |
|
The action runs on the pull_request event, so I can't test it in my own repo. When the actions are done, it will post a comment in this PR, let us wait and check whether it appears. |
bf75ae3 to
acf4fc4
Compare
|
Here is a example result of this PR The workflow in this PR is not working. Unless the PR is merged, otherwise github uses a read-only credential to run the proposal workflow. |
There was a problem hiding this comment.
Per the updated template, can you update the PR here?
#### Problem
What is being fixed? Examples:
* Fix crash on startup
* Fixes #12345 12345 Frobnozzle is leaky (exactly like that, so GitHub will auto-close the issue).
#### Change overview
What's in this PR
#### Testing
How was this tested? (at least one bullet point required)
• If unit tests were added, how do they cover this issue?
• If unit tests existed, how were they fixed/modified to prevent this in future?
• If integration tests were added, how do they verify this change?
• If manually tested, what platforms controller and device platforms were manually tested, and how?
• If no testing is required, why not?
@woody-apple Updated, thanks |
Yikes, |
This can be a separate issue. |
| # continue | ||
| if not comment.body.startswith(args.title): | ||
| continue | ||
| logging.info('Removing obsolete comment with heading "%s"', (args.title)) |
There was a problem hiding this comment.
This seems like a pretty sharp knife: if someone accidentally passes in empty string, this will delete all the comments, no?
I'd prefer it if all comments posted by this script had a known prefix from the script itself (which gets prepended to args.title).
Also, this is presumably copied in large part from bloat_check.py. How feasible would it be to make that script use this one?
There was a problem hiding this comment.
Let me re-enable the user id check, to see if we can guard this one
This comment has been minimized.
This comment has been minimized.
| - name: Report symbol diff | ||
| if: ${{ github.event_name == 'pull_request' }} | ||
| run: | | ||
| pip3 install cxxfilt pyelftools humanfriendly pandas && \ |
There was a problem hiding this comment.
should or could we install these packages in the docker image so that we do not run pip install on every commit?
We are trying to make CI faster. If possible, please create a separate PR that updates dockerfile and version.
There was a problem hiding this comment.
I updated scripts/requirements.in for this yesterday (#6975) so they should get in to future rolls.
|
@woody-apple the summary was updated to match the template |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
|
/rebase |
|
This was merged before we ensured PRs were not moved to red when we compute the difference (PRs fail on 'baseline download failed). Will revert @kghost |
…roject-chip#6962)" This reverts commit 1f1b715.
Problem
The PR add a github actions job to automatically post the memory difference of a new PR. It help us to diagnose memory usage.
Change overview
After building the new image, it download the prebuild base image from master branch, and use diffsyms tool to generate a diff, then call post_comment script to add a comment.
Testing
This is the example result of this PR kghost#3