-
Notifications
You must be signed in to change notification settings - Fork 13.3k
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
umm_malloc manual merge with upstream #7337
umm_malloc manual merge with upstream #7337
Conversation
… build. Correct overstepping array when freeing.
Comment corrections.
@devyte While working through memory boundary calculations on HeapMeteric.ino, I find myself questioning whether I should have done this. At the moment I am on the fence. |
I think not. The current behavior is that it's not possible to do malloc(maxBlock), which can be unexpected. However, from your explanations on gitter, even if you manage to make it work for maxBlock, it could potentially fail e. g. for maxBlock-overhead, because then the remaining space can't be divided. And that is also unexpected, which means the original cause for fixing it isn't resolved. |
contiguous block of memory before the umm_malloc overhead is removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After re-reading this a couple times it LGTM. The only non-debug path change I caught was the realloc that exactly fits (uncommon, so hard to test but on reading the code looks good). The debug/freeblock stuff I'm less worried about even though the changes are larger since it's not anywhere near as critical as the main malloc code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing jumps out at me
In
The upstream umm_malloc is a .c program and often does pointer math with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like UMM_INLINE_METRICS even it is still not enabled by default, maybe some more perf tests need to be performed.
Thanks for this fine work, thank you for keeping it sync with upstream.
Noteworthy Changes:
ESP.getHeapFragmentation()
is now calculated with blocks. Previously the calculation was done with bytes.ESP.getHeapFragmentation()
,-DUMM_INLINE_METRICS
would reduce long periods of interrupts disabled caused by frequent calls toumm_info()
. Instead, the computations get distributed across each malloc, realloc, and free. This appears to require an additional 116 bytes of IRAM vs usingUMM_STATS
withUMM_INFO
.UMM_STATS
andUMM_INLINE_METRICS
are defined, macros and structures have been optimized to reduce duplication.ESP.getFreeHeap()
,umm_free_heap_size()
,xPortGetFreeHeapSize()
,system_get_free_heap_size()
, ... previously reported values that were 8 bytes too large. In contrastESP.getMaxFreeBlockSize()
was not affected.ESP.getHeapStats(NULL, NULL, &hfrag);
with a fully allocated heap.New functions from upstream:
int umm_usage_metric( void )
, returns the ration of 100 * (used/free blocks). Returns -1 when free blocks is zero.int umm_fragmentation_metric( void )
, returns the same information asESP.getHeapFragmentation()
. WhenUMM_INLINE_METRICS
is definedESP.getHeapFragmentation()
is a thin wrapper toumm_fragmentation_metric()
.* Edited to reflect current PR content.