Skip to content

esp32: Add MICROPY_GC_INITIAL_HEAP_SIZE option and tune it. #13219

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
Dec 19, 2023

Conversation

dpgeorge
Copy link
Member

This PR consists of two commits.

First, there are changes to the GC to improve the calculation of the size of the next heap area when automatically expanding the heap:

  • Compute the existing total size by counting the total number of GC blocks, and then using that to compute the corresponding number of bytes.
  • Round the bytes value up to the nearest multiple of BYTES_PER_BLOCK.

This makes the calculation slightly simpler and more accurate, and makes sure that, in the case of growing from one area to two areas, the number of bytes allocated from the system for the second area is the same as the first. For example on esp32 with an initial area size of 65536 bytes, the subsequent allocation is also 65536 bytes. Previously it was a number that was not even a multiple of 2.

Second, a change to esp32 to add MICROPY_GC_INITIAL_HEAP_SIZE and tune its value. This gets back the old heap-size behaviour on ESP32, before auto-split-heap was introduced: after the heap is grown one time the size is 111936 bytes, with about 40k left for the IDF. That's enough to start WiFi and do a HTTPS request.

@dpgeorge
Copy link
Member Author

This is an alternative to #13035.

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
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS

Copy link

codecov bot commented Dec 18, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (3270d85) 98.40% compared to head (93c3b03) 98.40%.

❗ Current head 93c3b03 differs from pull request most recent head f6d6308. Consider uploading reports for the commit f6d6308 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #13219   +/-   ##
=======================================
  Coverage   98.40%   98.40%           
=======================================
  Files         159      159           
  Lines       21075    21075           
=======================================
  Hits        20738    20738           
  Misses        337      337           

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

@dpgeorge dpgeorge added this to the release-1.22.0 milestone Dec 18, 2023
@jimmo
Copy link
Member

jimmo commented Dec 19, 2023

LGTM

There are two main changes here to improve the calculation of the size of
the next heap area when automatically expanding the heap:
- Compute the existing total size by counting the total number of GC
  blocks, and then using that to compute the corresponding number of bytes.
- Round the bytes value up to the nearest multiple of BYTES_PER_BLOCK.

This makes the calculation slightly simpler and more accurate, and makes
sure that, in the case of growing from one area to two areas, the number
of bytes allocated from the system for the second area is the same as the
first.  For example on esp32 with an initial area size of 65536 bytes, the
subsequent allocation is also 65536 bytes.  Previously it was a number that
was not even a multiple of 2.

Signed-off-by: Damien George <damien@micropython.org>
This gets back the old heap-size behaviour on ESP32, before auto-split-heap
was introduced: after the heap is grown one time the size is 111936 bytes,
with about 40k left for the IDF.  That's enough to start WiFi and do a
HTTPS request.

Signed-off-by: Damien George <damien@micropython.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants