Skip to content

tools/metrics.py: Add VIRT_RV32 to the code size metrics. #15843

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

Merged
merged 2 commits into from
Sep 23, 2024

Conversation

agatti
Copy link
Contributor

@agatti agatti commented Sep 12, 2024

Summary

This commit adds the Qemu-based RISC-V 32 bits VIRT_RV32 board to the list of ports/boards to be built for measuring code size changes, as per the discussion in #15743 (comment).

Testing

Ran the tool locally on a CI-like environment.

Trade-offs and Alternatives

In the original discussion it was mentioned the potential for the metrics collection step to take too long. The RV32 port in question brings in as little third party code as possible and it should be relatively quick to build, all things considered (a single full build takes around a minute on an underclocked i5 laptop). However, until there are real measurements being taken in the final environment this is all speculation.

Copy link

codecov bot commented Sep 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.57%. Comparing base (8a9042a) to head (3e085c3).
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #15843   +/-   ##
=======================================
  Coverage   98.57%   98.57%           
=======================================
  Files         164      164           
  Lines       21341    21341           
=======================================
  Hits        21036    21036           
  Misses        305      305           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS

@dpgeorge
Copy link
Member

This won't actually be reported by the github-actions bot (see its comment immediately above) unless "v" is added to the PORTS_TO_CHECK list in tools/ci.sh:ci_code_size_build.

@agatti
Copy link
Contributor Author

agatti commented Sep 13, 2024

Sorry, I thought I added the second commit when I force updated the branch last night. The second commit is now part of the PR.

@dpgeorge
Copy link
Member

The CI for the code size check is failing, but I think that will only work once this is merged, so that it has the "v" option on master to check the PR against.

@dpgeorge dpgeorge added the tools Relates to tools/ directory in source, or other tooling label Sep 19, 2024
@dpgeorge
Copy link
Member

@agatti are you sure this will work, won't it need to install a RISC-V toolchain in ci_code_size_setup?

@agatti
Copy link
Contributor Author

agatti commented Sep 19, 2024

You're right, turned out I tested that with a VM that had already built the RV32 code once. This should now be addressed.

Edit: Wait, that runs on 20.04? Is it OK if I update the workflow to use 22.04 or is there a particular reason that workflow is still on that?

@dpgeorge
Copy link
Member

Wait, that runs on 20.04? Is it OK if I update the workflow to use 22.04 or is there a particular reason that workflow is still on that?

See commit 0a07915 ... something to do with gcc-multilib.

You're welcome to try updating that to 22.04, but it would need to be a separate PR because that CI step needs to be tested, and it won't be tested here because it fails due to missing "v" support.

@agatti
Copy link
Contributor Author

agatti commented Sep 20, 2024

That should be fixed in #15884 - besides the RV64 and macOS failures (expected and occasional), the code size task seems to work just fine on 22.04.

This commit adds the Qemu-based RISC-V 32 bits `VIRT_RV32` board to the
list of ports/boards to be built for measuring code size changes.

Signed-off-by: Alessandro Gatti <a.gatti@frob.it>
This commit adds the `VIRT_RV32` board to the list of targets for
calculating code size changes as part of the CI pipeline.

Signed-off-by: Alessandro Gatti <a.gatti@frob.it>
@dpgeorge dpgeorge merged commit 3e085c3 into micropython:master Sep 23, 2024
64 of 65 checks passed
@agatti agatti deleted the codesize-rv32 branch September 23, 2024 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tools Relates to tools/ directory in source, or other tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants