Skip to content

py/gc: Fix search hint for a full area in split-heap gc_alloc. - #19724

Open
bwhitman wants to merge 1 commit into
micropython:masterfrom
bwhitman:gc-split-heap-full-area-hint
Open

bwhitman wants to merge 1 commit into
micropython:masterfrom
bwhitman:gc-split-heap-full-area-hint

Conversation

@bwhitman

Copy link
Copy Markdown

When gc_alloc scans an area of a split heap to the end without finding a free block, it marks the area as full by setting its gc_last_free_atb_index, so that later scans skip it. But at that point i is an ATB byte index (the loop ran off the end of the table), and it was divided by BLOCKS_PER_ATB as if it were a block index. The hint therefore landed a quarter of the way into the table, and every later scan of that full area re-walked the other three quarters.

Such a scan happens each time gc_free releases a block in an area other than gc_last_free_area, which resets the search to the first area. On an ESP32-P4 with PSRAM and six heap areas, that re-walk was about 62 KB of allocation table per 16 ms frame, costing ~3.5 ms per frame in gc_alloc. With this fix the hint points past the end of the table.

Testing

Tested on ESP32-P4x.

Generative AI

I used generative AI tools when creating this PR, but a human has checked the code and is responsible for the code and the description above.

When gc_alloc scans an area of a split heap to the end without finding
a free block, it marks the area as full by setting its
gc_last_free_atb_index, so that later scans skip it.  But at that point
i is an ATB byte index (the loop ran off the end of the table), and it
was divided by BLOCKS_PER_ATB as if it were a block index.  The hint
therefore landed a quarter of the way into the table, and every later
scan of that full area re-walked the other three quarters.

Such a scan happens each time gc_free releases a block in an area other
than gc_last_free_area, which resets the search to the first area.  On
an ESP32-P4 with PSRAM and six heap areas, that re-walk was about 62 KB
of allocation table per 16 ms frame, costing ~3.5 ms per frame in
gc_alloc.  With this fix the hint points past the end of the table.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Brian Whitman <brian@variogram.com>
@bwhitman
bwhitman force-pushed the gc-split-heap-full-area-hint branch from 016ca2b to 9a336ab Compare September 24, 2026 01:01
@codecov

codecov Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.55%. Comparing base (09f5bb4) to head (9a336ab).
⚠️ Report is 10 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #19724      +/-   ##
==========================================
- Coverage   98.59%   98.55%   -0.04%     
==========================================
  Files         182      182              
  Lines       23335    23335              
  Branches        5        5              
==========================================
- Hits        23006    22998       -8     
- Misses        328      336       +8     
  Partials        1        1              
Flag Coverage Δ
unix-coverage-32bit 98.55% <100.00%> (-0.04%) ⬇️
unix-coverage-64bit 98.52% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

Copy link
Copy Markdown

Code size report:

Reference:  mimxrt/modmachine: Fix machine.deepsleep(ms) for the MIMXRT1176 port. [09f5bb4]
Comparison: py/gc: Fix search hint for a full area in split-heap gc_alloc. [merge of 9a336ab]
  mpy-cross:    +0 +0.000% 
   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
      esp32:    +0 +0.000% ESP32_GENERIC
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:    +0 +0.000% VIRT_RV32

@projectgus
projectgus self-requested a review September 24, 2026 02:02
@projectgus projectgus added the py-core Relates to py/ directory in source label Sep 30, 2026

@projectgus projectgus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good catch! This change looks good to me.

I feel like we could maybe save a little code size here (or maybe not) by doing:

            area->gc_last_free_atb_index = SIZE_MAX

i.e. the less hacky version of the suggested (size_t)-1 in the comment from 2018.

But either way it's OK.

@projectgus
projectgus requested a review from dpgeorge September 30, 2026 00:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

py-core Relates to py/ directory in source

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants