Skip to content

Add selective collect to memory allocations #10264

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

Merged
merged 8 commits into from
Apr 25, 2025

Conversation

tannewt
Copy link
Member

@tannewt tannewt commented Apr 16, 2025

By selectively collecting an allocation, we can skip scanning many allocations for pointers because we know up front they won't have them. This helps a ton when large buffers are being used and memory is slow (PSRAM). In one Fruit Jam example GC times drop from 80+ms to ~25ms. The example uses a number of bitmaps that are now no longer scanned.

@tannewt
Copy link
Member Author

tannewt commented Apr 17, 2025

I have this branch with debugging prints and checks: https://github.com/tannewt/circuitpython/tree/debug_gc_selective_collect

@tannewt
Copy link
Member Author

tannewt commented Apr 17, 2025

Hopefully #10269 will make enough space for this.

By selectively collecting an allocation, we can skip scanning many
allocations for pointers because we know up front they won't have
them. This helps a ton when large buffers are being used and memory is
slow (PSRAM). In one Fruit Jam example GC times drop from 80+ms to
~25ms. The example uses a number of bitmaps that are now no longer
scanned.
@tannewt tannewt force-pushed the gc_selective_collect branch from 69e4bcd to c340596 Compare April 22, 2025 21:08
Update Hallowing M4 so that it has synthio, jpegio and
tilepalettemapper. It doesn't need *target or the other display buses.
@tannewt tannewt requested a review from dhalbert April 23, 2025 17:59
@tannewt tannewt marked this pull request as ready for review April 23, 2025 17:59
Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

This is a big change but is a very good idea.

I tried to do complete // CIRCUITPY-CHANGE labelling, partly for my own benefit at the next upstream mege, and partly for the reader.

As I discussed in two comments, I think it might be safer to force conscious choice between the _with_collect() and _without_collect() versions of things by eliminating the old ambiguous name, or else make the old name do the safer thing of assuming collecting.

@tannewt tannewt force-pushed the gc_selective_collect branch from 4025752 to 35ffb64 Compare April 24, 2025 19:30
@tannewt
Copy link
Member Author

tannewt commented Apr 24, 2025

I've also modified MICROPY_ALLOC_GC_STACK_SIZE from 64 to 128. It alone drops GC times from 200+ms to ~110ms without selective collect. Selective collect without the stack increase drops to ~25ms. With the stack increase it drops further to ~15ms.

This is when testing https://github.com/adafruit/Adafruit_Learning_System_Guides/pull/3013/files on Fruit Jam (RP2350 with PSRAM).

@tannewt tannewt requested a review from dhalbert April 25, 2025 23:16
Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

Looks good! And the m_malloc_without_collect() is clearer.

@dhalbert dhalbert merged commit fa9b974 into adafruit:main Apr 25, 2025
54 checks passed
@AimSD23nSiR
Copy link

Jammed

@tannewt
Copy link
Member Author

tannewt commented Apr 28, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants