Skip to content

esp32: control the python heap size via NVS variables #6785

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

Closed
wants to merge 1 commit into from

Conversation

tve
Copy link
Contributor

@tve tve commented Jan 18, 2021

This PR builds on top of #6780 and #6782.

This commit allows the user to limit the micropython heap size by setting NVS variables. This supports:

  • ensuring that esp-idf has enough memory for multiple TLS connections, TLS plus BLE, etc.
  • leaving some SPIRAM memory for native modules, such as camera framebuffers
  • reducing the SPIRAM memory to limit GC sweep times

See the docs changes for more rationale and details.
A test script is included, but not usable with run-tests due to the resets required.

@tve tve force-pushed the esp32-set-heapsize branch from 5cb9aa0 to e272f26 Compare January 18, 2021 21:02
@pumelo
Copy link

pumelo commented Jan 24, 2021

Cool, like it.
What is the maximum heap size you can allocate for mp with this method? I briefly recall, that I have tried something similar but ended up with a much smaller max heap size for mp than when just using all SPIRAM. What is your expirience?

@tve
Copy link
Contributor Author

tve commented Feb 4, 2021

What is the maximum heap size you can allocate for mp with this method?

The max size is what you get today and continues to be the same if you do not set any NVS variable. This PR only allows you to reduce the heap from the default. It does not allow you to grab more memory. (MP already grabs the max it can...)

@pumelo
Copy link

pumelo commented Feb 4, 2021

If you follow this code path starting from here:

#if CONFIG_ESP32_SPIRAM_SUPPORT || CONFIG_SPIRAM_SUPPORT

Here malloc is not used but the whole spiram assigned to mp. using malloc results in a much smaller amount of heap assigned to mp (at least in my expirience). That was the reason asking.

@tve
Copy link
Contributor Author

tve commented Feb 4, 2021

If you follow this code path starting from here:

#if CONFIG_ESP32_SPIRAM_SUPPORT || CONFIG_SPIRAM_SUPPORT

Here malloc is not used but the whole spiram assigned to mp. using malloc results in a much smaller amount of heap assigned to mp (at least in my expirience). That was the reason asking.

The trick is CONFIG_SPIRAM_USE_CAPS_ALLOC=y. Before, the SPIRAM was hidden from esp-idf's malloc. With the changes it no longer is.

@dpgeorge
Copy link
Member

Can you please rebase this on latest master? All prerequisites are now merged.

@tve tve force-pushed the esp32-set-heapsize branch from 5e81d99 to 33bbe02 Compare March 2, 2021 05:39
@tve tve marked this pull request as draft March 2, 2021 05:39
@tve tve force-pushed the esp32-set-heapsize branch 2 times, most recently from 6d145b8 to c17912d Compare March 2, 2021 05:54
This commit allows the user to limit the micropython heap size by setting
NVS variables. This supports:
- ensuring that esp-idf has enough memory for multiple TLS connections,
  TLS plus BLE, etc.
- leaving some SPIRAM memory for native modules, such as camera
  framebuffers
- reducing the SPIRAM memory to limit GC sweep times

See the docs changes for more details.
@tve tve force-pushed the esp32-set-heapsize branch from c17912d to 865051f Compare March 2, 2021 16:18
@tve
Copy link
Contributor Author

tve commented Mar 2, 2021

Rebased against current master and manually tested with the provided test script and examples from the docs against esp-idf v4.2 for the GENERIC and TINYPICO board variants.

@tve tve marked this pull request as ready for review March 2, 2021 16:38
}
void *mp_task_heap = heap_caps_malloc(mp_task_heap_size, MALLOC_CAP_8BIT);
if (avail_heap_size != mp_task_heap_size) {
printf("Heap limited: avail=%d, actual=%d, left to idf=%d\n",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO it should not print anything here. The user can programatically inspect the heap sizes and print their own message if needed.

@@ -190,6 +190,7 @@ STATIC const mp_rom_map_elem_t esp32_module_globals_table[] = {
{ MP_ROM_QSTR(MP_QSTR_Partition), MP_ROM_PTR(&esp32_partition_type) },
{ MP_ROM_QSTR(MP_QSTR_RMT), MP_ROM_PTR(&esp32_rmt_type) },
{ MP_ROM_QSTR(MP_QSTR_ULP), MP_ROM_PTR(&esp32_ulp_type) },
{ MP_ROM_QSTR(MP_QSTR_NVS), MP_ROM_PTR(&esp32_nvs_type) },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not needed

@@ -30,5 +30,6 @@ extern const mp_obj_type_t esp32_nvs_type;
extern const mp_obj_type_t esp32_partition_type;
extern const mp_obj_type_t esp32_rmt_type;
extern const mp_obj_type_t esp32_ulp_type;
extern const mp_obj_type_t esp32_nvs_type;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not needed

@dpgeorge
Copy link
Member

This is a useful feature. But I'm wondering if it could be done using normal Python function calls rather than NVS settings, which would then be generalisable to other ports that may want this feature.

The reason stm32 had boot.py and main.py split from the beginning was to configure things like this (USB settings, enable certain hardware features) which affect the system in a significant way. For example we could add something like:

micropython.resize_heap(lower_bound, upper_bound)

which restarts the whole system with a resized heap if its size is not already within the given bounds. (And maybe eventually this function could be used it increase heap size on the fly, eg on unix where it can ask the OS for more memory.)

In the end, both NVS settings and Python function calls scale well to many configuration settings. It's just that Python function calls are more Pythonic and could be used on other ports.

@jimmo
Copy link
Member

jimmo commented Nov 15, 2022

We'd like to solve this using an approach similar to described above with boot.py. In particular we now support split heap, so the heap can grow at runtime.
Ref #8940 for more background on IDF heap issues.

@tve
Copy link
Contributor Author

tve commented Nov 21, 2022

For reference: split heap PR #8526

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.

4 participants