Optimize allocation of managed memory #2940
Conversation
WalkthroughThe changes primarily involve memory management adjustments and API updates for the ESP32 target. Key modifications include removing the Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant ESP32
participant MemoryManager
User ->> ESP32: Start Application
ESP32 ->> MemoryManager: Initialize Memory
MemoryManager -->> ESP32: Allocate Managed Heap
ESP32 ->> MemoryManager: Configure Bluetooth and Thread
MemoryManager -->> ESP32: Reserve Extra Memory
ESP32 ->> User: Application Running
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- targets/ESP32/CMakePresets.json (3 hunks)
- targets/ESP32/_nanoCLR/Memory.cpp (1 hunks)
Files skipped from review due to trivial changes (1)
- targets/ESP32/CMakePresets.json
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- targets/ESP32/_nanoCLR/Memory.cpp (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- targets/ESP32/_nanoCLR/Memory.cpp
AdrianSoundy
added
the
Platform: ESP32
AdrianSoundy
force-pushed
the
C3-managed-memory
branch
from
c59a96b
to
8463b1e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- targets/ESP32/CMakePresets.json (16 hunks)
- targets/ESP32/_nanoCLR/Memory.cpp (4 hunks)
Files skipped from review due to trivial changes (1)
- targets/ESP32/CMakePresets.json
Additional context used
Learnings (1)
targets/ESP32/_nanoCLR/Memory.cpp (1)
User: AdrianSoundy PR: nanoframework/nf-interpreter#2940 File: targets/ESP32/_nanoCLR/Memory.cpp:140-140 Timestamp: 2024-05-18T23:04:15.581Z Learning: In the `targets/ESP32/_nanoCLR/Memory.cpp` file, the `largestFreeBlock` value is already logged in the message: `ESP_LOGI(TAG, "Managed heap allocated: %d (from max:%d)", managedHeapSize, largestFreeBlock);`.
Additional comments not posted (3)
targets/ESP32/_nanoCLR/Memory.cpp (3)
24-28: Adjust theMINIMUM_FREE_MEMORY_FOR_IDFbased on the target.This change allows for different memory reservations based on the ESP32 model, which is a sensible approach given the varying capabilities of different hardware versions.
31-33: Define extra memory reserves for Bluetooth and Thread configurations.These definitions are clear and align with the need to reserve additional memory when specific features are enabled, ensuring that the system remains stable and functional under these configurations.
169-169: Enhance logging for better traceability.The log message provides valuable information about the allocation process, enhancing traceability and debugging capabilities.
|
@josesimoes The new algorithm uses the free native memory instead to get a more accurate idea of available memory. Note these changes only affect native memory allocation to IRAM and not the original SPIRAM allocation To help with this all the ESP32_RESERVED_RAM_FOR_IDF_ALLOCATION were removed from CmakePresets.json. A lot of the original entries didn't do anything due to heap fragmentation. TestingFrom testing comparing IDF 4 -> IDF 5 for various targets S3 target are not listed as normally using PSRAM
|
|
Available memory looks much better today for most targets! |
AdrianSoundy
changed the title
|
Yay, thankyou master Soundy! |
Description
For some reason we were getting a much smaller size of managed heap when running on IF 5.x firmware then previously even through parameters where unchanged. To try to correct this another PR was merged with a changed ESP32_RESERVED_RAM_FOR_IDF_ALLOCATION parameter (changed from 100 -> 50). This improved memory size but not enough.
I have investigated why this happened and adjusted code to stop it happening on future releases.
The reason is due to the IDF 5.x version having a more fragmented heap although still a similar amount of free memory.
We were working out how much memory to allocate for managed heap based on largest free block minus amount of memory to reserve for IDF.
This has now been changed to use the free space instead of largest free then sized to fit largest block.
This is a more accurate value to reserve for IDF. The CmakePreset.json values have been restored to original values.
Largest free block with IDF 4.4.6 was 221184. With IDF 5.1.3 it is 114688
Free heap for 4.4.6 is 240600 and 232884 for IDF 5.1.3
Motivation and Context
Issue reported on discord
How Has This Been Tested?
Tested with local builds and c# test program.
Types of changes
Checklist
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
ESP32_RESERVED_RAM_FOR_IDF_ALLOCATIONparameter for cleaner configuration.