Skip to content

py/gc & esp32: Include largest free block in gc.mem_free() if MICROPY_GC_SPLIT_HEAP_AUTO is enabled - #12366

Merged
dpgeorge merged 3 commits into
micropython:masterfrom
projectgus:feature/esp32_mem_free_largest_free_block
Sep 15, 2023
Merged

py/gc & esp32: Include largest free block in gc.mem_free() if MICROPY_GC_SPLIT_HEAP_AUTO is enabled#12366
dpgeorge merged 3 commits into
micropython:masterfrom
projectgus:feature/esp32_mem_free_largest_free_block

Conversation

@projectgus

@projectgus projectgus commented Sep 5, 2023

Copy link
Copy Markdown
Contributor

This is a follow-up to #12141 based on discussions in #12316, and changes the behaviour of the esp32 port only.

  • Add the "max new split" value to the result of gc.mem_free(), to give a more accurate impression of how much RAM is available for Python code to use.
  • Update esp32 docs to match, and clarify that gc.mem_alloc() and gc.mem_free() functions are relevant for Python allocations only.

This work was funded through GitHub Sponsors.

@projectgus projectgus added port-esp32 py-core Relates to py/ directory in source labels Sep 5, 2023
@projectgus
projectgus requested review from dpgeorge and jimmo September 5, 2023 01:14
@projectgus projectgus self-assigned this Sep 5, 2023
@github-actions

github-actions Bot commented Sep 5, 2023

Copy link
Copy Markdown

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

@codecov

codecov Bot commented Sep 5, 2023 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #12366 (00f13e4) into master (174bb28) will decrease coverage by 0.01%.
The diff coverage is n/a.

❗ Current head 00f13e4 differs from pull request most recent head b0da5cb. Consider uploading reports for the commit b0da5cb to get more accurate results

@@            Coverage Diff             @@
##           master   #12366      +/-   ##
==========================================
- Coverage   98.38%   98.38%   -0.01%     
==========================================
  Files         158      158              
  Lines       20940    20933       -7     
==========================================
- Hits        20602    20595       -7     
  Misses        338      338
Files Changed Coverage Δ
py/gc.c 99.22% <ø> (ø)
py/modgc.c 100.00% <ø> (ø)

... and 4 files with indirect coverage changes

@bulletmark

bulletmark commented Sep 11, 2023

Copy link
Copy Markdown
Contributor

Note to reviewers. I made the comments in #12316 which prompted this change. So I downloaded this PR as a patch and git applied it to the latest MP tree which I then tested across a couple of different ESP32 boards and can confirm that it works well.

jimmo
jimmo approved these changes Sep 11, 2023
View reviewed changes

@jimmo jimmo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @projectgus - I like the way you've kept it as a separate field in gc_info as well as the mem_info output.

I'm a little bit iffy on the "by Python code" docs change because it might give the wrong impression, but overall I think it's a net positive change and instead the time could be better spent on a more comprehensive guide to the way memory works in MicroPython that can explain the nuances with more context.

@projectgus

projectgus commented Sep 11, 2023

Copy link
Copy Markdown
Contributor Author

more comprehensive guide to the way memory works in MicroPython that can explain the nuances with more context.

I think that would be really useful!

@dpgeorge
dpgeorge force-pushed the feature/esp32_mem_free_largest_free_block branch from 00f13e4 to b0da5cb Compare September 15, 2023 02:16
@projectgus @dpgeorge
py/gc: Add "max new split" value in result of gc.mem_free().
3e8aed9 
Follow-up to 519c24d when MICROPY_GC_SPLIT_HEAP_AUTO is enabled, based
on discussion at
https://github.com/orgs/micropython/discussions/12316#discussioncomment-6858007

gc.mem_free() is always a heuristic, but this makes it a more useful
heuristic for common use cases.

Signed-off-by: Angus Gratton <angus@redyak.com.au>
@projectgus @dpgeorge
docs/library/esp32: Update ESP32 idf_heap_info docs to match behaviour.
26160e8 
Signed-off-by: Angus Gratton <angus@redyak.com.au>
@projectgus @dpgeorge
docs/library/gc: Clarify mem_alloc and mem_free only for Python heap.
92f379c 
As raised in discussions of the ESP32 memory management changes.

Signed-off-by: Angus Gratton <angus@redyak.com.au>
@dpgeorge
dpgeorge force-pushed the feature/esp32_mem_free_largest_free_block branch from b0da5cb to 92f379c Compare September 15, 2023 02:19
dpgeorge
dpgeorge reviewed Sep 15, 2023
View reviewed changes
Comment thread py/gc.c
@dpgeorge
dpgeorge merged commit 92f379c into micropython:master Sep 15, 2023
@projectgus
projectgus deleted the feature/esp32_mem_free_largest_free_block branch September 19, 2023 02:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dpgeorge dpgeorge dpgeorge left review comments
@jimmo jimmo jimmo approved these changes

Assignees

@projectgus projectgus

Labels

port-esp32 py-core Relates to py/ directory in source

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@projectgus @bulletmark @jimmo @dpgeorge