-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
py/gc: Support multiple heaps (version 3). #8526
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
Conversation
eb1cf5f
to
2d5128a
Compare
CI sometimes has spurious failures (eg on |
2d5128a
to
f1d9156
Compare
Yea there were a few unix port variants failing that had been passing, which was due to a quick refactor. Issue is fixed now. The only variant failing now is macos, due to singlee test: repl_inspect. Any idea what could be causing that? |
Codecov Report
@@ Coverage Diff @@
## master #8526 +/- ##
==========================================
- Coverage 98.25% 98.20% -0.05%
==========================================
Files 154 154
Lines 20288 20337 +49
==========================================
+ Hits 19933 19971 +38
- Misses 355 366 +11
Continue to review full report at Codecov.
|
I haven't seen --- /Users/runner/work/micropython/micropython/tests/results/cmdline_repl_inspect.py.exp 2022-04-13 02:23:06.000000000 +0000
+++ /Users/runner/work/micropython/micropython/tests/results/cmdline_repl_inspect.py.out 2022-04-13 02:23:06.000000000 +0000
@@ -1,6 +1,3 @@
test
-MicroPython \.\+ version
-Use \.\+
->>> # cmdline: -i -c print("test")
->>> # -c option combined with -i option results in REPL
->>>
+# cmdline: -i -c print("test")
+# -c option combined with -i option results in REPL I don't know... maybe push again to re-trigger CI and see if it happens again. |
5a953e9
to
96063b8
Compare
Enable the addition of heap space at runtime. Advantages: - The ESP32 has a fragmented heap so to use all of it the heap must be split. - Support a dynamic heap while running on an OS, adding more heap when necessary. Original commit from Jan 2018. Rebased onto latest master.
96063b8
to
d0bbd2a
Compare
Use C macros to reduce size of firmware images when GC multiple heap feature is disabled. Image size diffs versus current master (28e7e15) when multi-heap *disabled*: bare-arm: +0 +0.000% minimal x86: +0 +0.000% unix x64: -16 -0.003% unix nanbox: -20 -0.004% stm32: -8 -0.002% PYBV10 cc3200: +0 +0.000% esp8266: +8 +0.001% GENERIC esp32: +0 +0.000% GENERIC nrf: -20 -0.011% pca10040 rp2: +0 +0.000% PICO samd: -4 -0.003% ADAFRUIT_ITSYBITSY_M4_EXPRESS Note that only the stm32 image gets slightly bigger (+8). Image size diffs versus current master when multi-heap is *enabled* with `MICROPY_GC_MULTIHEAP=1` (but not extra code to add more heaps): unix x64: +1032 +0.197% [incl +544(bss)] esp32: +592 +0.039% GENERIC[incl +16(data) +264(bss)]
When multiple GC heaps are enabled for the unix port, the value of the C macro MICROPY_GC_N_HEAPS determines over how many heaps the requested heap size is divided. The coverage variant sets this to 4 to cover the `gc_add()` function and test the multiple heaps feature. The default is 1.
d0bbd2a
to
691fca8
Compare
Would this enable a fix for #8428? |
Yes. The aim with multi-heap is that the system can add more dynamically. Eg on the unix port it can expand the heap as necessary. Same for any port that has lots of RAM. |
When multi-heaps are enabled an array of pointers is used to track the heap areas that correspond to each block on the gc stack. This causes an increase in uninitialized memory (bss) in the image sizes . Pointers to heap areas are used because the heap areas themselves are stored using a linked list. Firmware size diff for multi-heap enabled versus disabled when heap areas are stored with a linked list (the current PR): unix x64: +1048 +0.200% [incl +544(bss)]
esp32: +592 +0.039% GENERIC[incl +16(data) +264(bss)] I've experimented with reducing the image sizes by storing the heap areas using an array instead of a linked list, so that only a byte array ( Firmware size diff for multi-heap enabled versus disabled when heap areas stored with an array: unix x64: +1512 +0.289% [incl +128(bss)]
esp32: +892 +0.059% GENERIC[incl +96(bss)] It's likelythe array implementation can be optimised further (happy to share a link to this branch), but my impression is that the total gain will be minimal (if anything). Seeing as the firmware sizes across different ports when multi-heap is disabled essentially stay the same, I propose to merge the current (linked list) implementation. |
Running performance test of this PR on PYBv1.0, the diff compared to master is:
|
Rebased and merged in bcc827d through d2e4cf0, with a rename of the new option to This is a great feature, thank you @aykevl and @rknegjens ! |
Hopefully this will lead to progress on #8428 (comment). |
Is there any documentation on how any of this works? What do I need to enable? Is it enabled by default on which ports? What does it really do, e.g., what is the starting heap size? When does it add more heap segments? How can one limit this, e.g. reserve a specific amt of memory for other stuff, like ESP-IDF? Can heaps be freed ever? |
The documentation available at the moment is in the code, the use of this feature by the unix port (see
Enable
Only unix coverage enables it.
The starting heap size is set when
When
It's up to the port (C code) to implement this logic.
No. |
@dpgeorge how to enable this for esp32 |
See my comment directly above on how to use this feature. |
Fix typo in TCP server test readme.md
Update of PR 3580 from @aykevl that enables addition of heap space at runtime. See the original PR for background and motivation.
This PR rebases the original onto latest master, integrating updates to the GC since then (Jan 2018). Unix port is passing tests with and without multiple heaps enabled.
It also adds some C macro optimisations to reduce firmware image sizes when multiple heaps are disabled.
Image size diffs versus current master (28e7e15) when multi-heap disabled:
Note that only the esp8266 image gets slightly bigger (+8).
Image size diffs versus current master when multi-heap is enabled with
MICROPY_GC_MULTIHEAP=1
(but not extra code to add more heaps):