-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
Allocate less micropython heap and make more space for IDF heap in ESP32-S2 port #7214
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
Comments
This might be fixed by #7163. |
#7163 fixes it only for devices with SPIRAM, because it will then use SPIRAM for the heap. |
Still no progress :-( @dpgeorge @robert-hh I'd like to make a PR, but I like to get some comments on proposed heap size calculation. |
This issue specifically mentions ESP32-S2, but I think we need to start with plain ESP32 for which:
So there's already something there to fix, just dealing with different IDF versions. The things that make it difficult are:
To solve (1) we need #3580, then the MicroPython heap can take as much or as little heap as it likes, it doesn't matter how much it's fragmented within the IDF (to an extent). To solve (4) we need a way for the user to specify the amount of heap to allocate, eg #6785. An alternative to #6785 would be to create an initial small heap (say 8k) which is used to run To solve (2) and (3) we need a sensible default value (or ratio) for MicroPython heap vs IDF free heap vs IDF used heap (already used by the time MicroPython starts). As proposed above by @tom-van, the value of 1/3 for MicroPython heap out of total heap does seem like a good sensible value, although it does give less heap than the original 111168 that ESP32 used to have. A lot of this was already mentioned in #7963, so I guess this is just a summary of things. We don't need to solve everything now. I think the idea proposed above of 1/3 is very reasonable and a good step forward. |
So that there is some memory left for the OS, eg for ssl buffers. See issue #7214. Signed-off-by: Damien George <damien@micropython.org>
In 23b1a4e I made it so that MicroPython allocates at most 1/2 of the total IDF heap. This makes sure 111168 bytes are still available on standard ESP32 with IDF 4.2. |
@dpgeorge thanks for addressing the issue! I confirm that 23b1a4e makes webrepl working on GENERIC_S2
The good point the problem is correctly reported, no cryptic WiFi disconnect happens this time. Just for those who want TLS functionality on S2 now I refer to fine tuned heap allocation I was working on I'm looking forward to some dynamic way to partition the heap. |
* Except for circuitpython.local which depends on MDNS and will be done in a follow up PR. Progress on micropython#7214
* Except for circuitpython.local which depends on MDNS and will be done in a follow up PR. Progress on micropython#7214
* Except for circuitpython.local which depends on MDNS and will be done in a follow up PR. Progress on micropython#7214
This adds both cpy-MAC.local and circuitpython.local support. Fixes micropython#7214
So that there is some memory left for the OS, eg for ssl buffers. See issue micropython#7214. Signed-off-by: Damien George <damien@micropython.org>
In the current micropython git master we get following memory in an ordinary ESP32 without SPI RAM right after boot:
so we have 62496 bytes available for python, 137132 for C malloc()
The changed memory layout of ESP32-S2 changes the ratio strongly in favour of micropython heap:
So on S2 we have 125360 bytes available for python and only 51976 bytes in IDF heaps for C malloc()
This gets exhausted just by connecting WiFi (STA mode with WPA2 PSK) and WebREPL request from network.
IMO we need to adjust mp_task_heap_size at ports/esp32/main.c:124
to better reflect the lower total amount of memory.
Something like
It does not change allocation on ESP32. On ESP32-S2 it allocates 71872 for python/109464 free for malloc, it's enough for WebREPL and upip.install('logging') works too. idf_heap_info() shows minimal seen size 25868, so we have some margin.
Or use some other percentage of the total size?
What do you think?
The text was updated successfully, but these errors were encountered: