From 5319a9a77bc67d893bb892286e442026cfa85d39 Mon Sep 17 00:00:00 2001 From: M Hightower <27247790+mhightower83@users.noreply.github.com> Date: Sun, 25 Sep 2022 08:15:07 -0700 Subject: [PATCH 1/6] Ensure xPortGetFreeHeapSize reports DRAM Create dedicated function for `xPortGetFreeHeapSize()` that only reports on DRAM. Update and export `umm_free_heap_size()` to report the free Heap space of the current Heap. Updated ESP.getFreeHeap() to use. Replace internal function `umm_free_heap_size_lw()` with `umm_free_heap_size()`. See umm_malloc/Notes.h entry Sep 26, 2022 for more specifics. uncrustified `umm_poison.c` --- cores/esp8266/Esp.cpp | 2 +- cores/esp8266/umm_malloc/Notes.h | 68 +++++++++++++++++++++++ cores/esp8266/umm_malloc/umm_info.c | 33 ++++++++++- cores/esp8266/umm_malloc/umm_local.c | 55 ++++++++++++++---- cores/esp8266/umm_malloc/umm_malloc_cfg.h | 8 +-- cores/esp8266/umm_malloc/umm_poison.c | 2 +- 6 files changed, 150 insertions(+), 18 deletions(-) diff --git a/cores/esp8266/Esp.cpp b/cores/esp8266/Esp.cpp index 8cddd13353..400bb10800 100644 --- a/cores/esp8266/Esp.cpp +++ b/cores/esp8266/Esp.cpp @@ -219,7 +219,7 @@ uint16_t EspClass::getVcc(void) uint32_t EspClass::getFreeHeap(void) { - return system_get_free_heap_size(); + return umm_free_heap_size(); } uint32_t EspClass::getMaxFreeBlockSize(void) diff --git a/cores/esp8266/umm_malloc/Notes.h b/cores/esp8266/umm_malloc/Notes.h index 33b2fc545a..2cd2d88fa6 100644 --- a/cores/esp8266/umm_malloc/Notes.h +++ b/cores/esp8266/umm_malloc/Notes.h @@ -276,4 +276,72 @@ Enhancement ideas: #endif ``` */ + +/* + Sep 26, 2022 + + History/Overview + + ESP.getFreeHeap() needs a function it can call for free Heap size. The legacy + method was the SDK function `system_get_free_heap_size()` which is in IRAM. + + `system_get_free_heap_size()` calls `xPortGetFreeHeapSize()` to get free heap + size. Our old legacy implementation used umm_info(), employing a + time-consuming method for getting free Heap size and runs with interrupts + blocked. + + Later we used a distributed method that maintained the free heap size with + each malloc API call that changed the Heap. (enabled by build option UMM_STATS + or UMM_STATS_FULL) We used an internally function `umm_free_heap_size_lw()` to + report free heap size. We satisfied the requirements for + `xPortGetFreeHeapSize()` with an alias to `umm_free_heap_size_lw()` + in replacement for the legacy umm_info() call wrapper. + + The upstream umm_malloc later implemented a similar method enabled by build + option UMM_INLINE_METRICS and introduced the function `umm_free_heap_size()`. + + The NONOS SDK alloc request must use the DRAM Heap. Need to Ensure DRAM Heap + results when multiple Heap support is enabled. Since the SDK uses portable + malloc calls pvPortMalloc, ... we leveraged that for a solution - force + pvPortMalloc, ... APIs to serve DRAM only. + + In an oversight, `xPortGetFreeHeapSize()` was left reporting the results for + the current heap selection via `umm_free_heap_size_lw()`. Thus, if an SDK + function like os_printf_plus were called when the current heap selection was + IRAM, it would get the results for the IRAM Heap. Then would receive DRAM with + an alloc request. However, when the free IRAM size is too small, it would + skip the Heap alloc request and use stack space. + + Solution + + The resolution is to rely on build UMM_STATS(default) or UMM_STATS_FULL for + free heap size information. When not available in the build, fallback to the + upstream umm_malloc's `umm_free_heap_size()` and require the build option + UMM_INLINE_METRICS. Otherwise, fail the build. + + Use function name `umm_free_heap_size()` to support external request for + current heap size. + + For the multiple Heap case `xPortGetFreeHeapSize()` becomes a unique function + and reports only DRAM free heap size. Now `system_get_free_heap_size()` will + always report DRAM free Heap size. This might be a breaking change. + + Specifics: + + * rename `umm_free_heap_size_lw()` to `umm_free_heap_size()` for builds that + include option UMM_STATS(default) or UMM_STATS_FULL. + * rename upstream umm_malloc's `umm_free_heap_size()` to + `umm_free_heap_size_info()`. + + * When the build options UMM_STATS/UMM_STATS_FULL are not used, fallback to + the upstream umm_malloc's original `umm_free_heap_size()` function in + umm_info.c + * require the UMM_INLINE_METRICS build option. + + * `xPortGetFreeHeapSize()` + * For single heap builds, alias to `umm_free_heap_size()` + * For multi-heap builds, add a dedicated function that always reports + DRAM results. + +*/ #endif diff --git a/cores/esp8266/umm_malloc/umm_info.c b/cores/esp8266/umm_malloc/umm_info.c index 4a95e994c3..d395fc5c29 100644 --- a/cores/esp8266/umm_malloc/umm_info.c +++ b/cores/esp8266/umm_malloc/umm_info.c @@ -174,7 +174,38 @@ size_t umm_free_heap_size_core(umm_heap_context_t *_context) { return (size_t)_context->info.freeBlocks * sizeof(umm_block); } -size_t umm_free_heap_size(void) { +/* + Need to expose a function for getting the current free heap size + Using umm_free_heap_size for this purpose. + + For an expanded discussion see Notes.h, entry dated "Sep 26, 2022" +*/ +#if defined(UMM_STATS) || defined(UMM_STATS_FULL) +/* + For this build option, see umm_free_heap_size in umm_local.c +*/ + +/* + Make upstream logic available under a new name. + May be useful for sanity checks between methods. + Only takes .bin space if used. +*/ +size_t umm_free_heap_size_info(void) + +#else +/* + Not our default build path. For this path to function well with WiFi enabled, + you must use the build option UMM_INLINE_METRICS. Otherwise, umm_info() is + used to complete the operation, which uses a time-consuming method for getting + free Heap and runs with interrupts off. Also, it cannot support calls from + ISRs, `umm_info()` runs from flash. + + Current build test in umm_local.c fails the build option that uses the + umm_info() method. +*/ +size_t umm_free_heap_size(void) +#endif +{ #ifndef UMM_INLINE_METRICS umm_info(NULL, false); #endif diff --git a/cores/esp8266/umm_malloc/umm_local.c b/cores/esp8266/umm_malloc/umm_local.c index a28fd16d93..c842214e80 100644 --- a/cores/esp8266/umm_malloc/umm_local.c +++ b/cores/esp8266/umm_malloc/umm_local.c @@ -166,28 +166,61 @@ size_t umm_block_size(void) { } #endif + #if defined(UMM_STATS) || defined(UMM_STATS_FULL) -// Keep complete call path in IRAM -size_t umm_free_heap_size_lw(void) { +// Support a consistant external name across build options. Used by +// ESP.getFreeHeap(). Keep complete call path in IRAM. +size_t umm_free_heap_size(void) { UMM_CHECK_INITIALIZED(); umm_heap_context_t *_context = umm_get_current_heap(); return (size_t)_context->UMM_FREE_BLOCKS * sizeof(umm_block); } -#endif +#elif defined(UMM_INLINE_METRICS) /* - I assume xPortGetFreeHeapSize needs to be in IRAM. Since - system_get_free_heap_size is in IRAM. Which would mean, umm_free_heap_size() - in flash, was not a safe alternative for returning the same information. + Use `size_t umm_free_heap_size(void)` (in umm_info.c) from upstream + umm_malloc. It must have the UMM_INLINE_METRICS Build option enabled to + support free heap size reporting without the use of `umm_info()` */ -#if defined(UMM_STATS) || defined(UMM_STATS_FULL) -size_t xPortGetFreeHeapSize(void) __attribute__ ((alias("umm_free_heap_size_lw"))); -#elif defined(UMM_INFO) -#ifndef UMM_INLINE_METRICS -#warning "No ISR safe function available to implement xPortGetFreeHeapSize()" +#else +// We require a resources to track and report free heap with low overhead. +// See umm_free_heap_size in umm_info.c for more details +#error UMM_INLINE_METRICS, UMM_STATS, or UMM_STATS_FULL needs to be defined. #endif + +/* + This API is called by `system_get_free_heap_size()` which is in IRAM. Use IRAM + to ensure the call chain is in IRAM. Which implies it may be called from an + ISR. + + To satisfy this requirement, we need UMM_STATS... or UMM_INLINE_METRICS + defined. These support an always available without intense computation + free-Heap value. + + Like the other vPort... APIs used by the SDK, this must always report on the + DRAM Heap not current Heap. +*/ +#if (UMM_NUM_HEAPS == 1) +// Reduce IRAM usage for the single Heap case size_t xPortGetFreeHeapSize(void) __attribute__ ((alias("umm_free_heap_size"))); + +#else +size_t xPortGetFreeHeapSize(void) { + UMM_CHECK_INITIALIZED(); + umm_heap_context_t *_context = umm_get_heap_by_id(UMM_HEAP_DRAM); + + #if defined(UMM_STATS) || defined(UMM_STATS_FULL) + return (size_t)_context->UMM_FREE_BLOCKS * sizeof(umm_block); + #elif defined(UMM_INLINE_METRICS) + return umm_free_heap_size_core(_context); + #else + // At this time, this build path is not reachable. In case things change, + // keep build check. + // Not in IRAM umm_info() has to be used to complete this operation. + #error "No ISR safe function available to implement xPortGetFreeHeapSize()" + #endif +} #endif #if defined(UMM_STATS) || defined(UMM_STATS_FULL) diff --git a/cores/esp8266/umm_malloc/umm_malloc_cfg.h b/cores/esp8266/umm_malloc/umm_malloc_cfg.h index 26c9c05563..9b756faa08 100644 --- a/cores/esp8266/umm_malloc/umm_malloc_cfg.h +++ b/cores/esp8266/umm_malloc/umm_malloc_cfg.h @@ -213,11 +213,11 @@ struct UMM_HEAP_CONTEXT; typedef struct UMM_HEAP_CONTEXT umm_heap_context_t; extern ICACHE_FLASH_ATTR void *umm_info(void *ptr, bool force); -#ifdef UMM_INLINE_METRICS -extern size_t umm_free_heap_size(void); -#else -extern ICACHE_FLASH_ATTR size_t umm_free_heap_size(void); +#if (defined(UMM_STATS) || defined(UMM_STATS_FULL)) && defined(UMM_INFO) +extern ICACHE_FLASH_ATTR size_t umm_free_heap_size_info(void); #endif +extern size_t umm_free_heap_size(void); + // umm_max_block_size changed to umm_max_free_block_size in upstream. extern ICACHE_FLASH_ATTR size_t umm_max_block_size(void); extern ICACHE_FLASH_ATTR int umm_usage_metric(void); diff --git a/cores/esp8266/umm_malloc/umm_poison.c b/cores/esp8266/umm_malloc/umm_poison.c index dc9d5322bf..9ebaafdd7a 100644 --- a/cores/esp8266/umm_malloc/umm_poison.c +++ b/cores/esp8266/umm_malloc/umm_poison.c @@ -15,7 +15,7 @@ * If `s` is 0, returns 0. * If result overflows/wraps, return saturation value. */ -static void add_poison_size(size_t* s) { +static void add_poison_size(size_t *s) { if (*s == 0) { return; } From 7af81082ccdcb02d7c6fb2586d07e2a02cd7d65e Mon Sep 17 00:00:00 2001 From: M Hightower <27247790+mhightower83@users.noreply.github.com> Date: Wed, 28 Sep 2022 20:16:06 -0700 Subject: [PATCH 2/6] Updated function names and alias usage. While touching and regression testing of build macros for umm_info... and umm_stats..., improved/fixed macros for better support of build options specified through Sketch.ino.globals.h. --- cores/esp8266/Esp.cpp | 2 +- cores/esp8266/umm_malloc/Notes.h | 20 ++- cores/esp8266/umm_malloc/umm_info.c | 37 ++---- cores/esp8266/umm_malloc/umm_local.c | 61 ++++++--- cores/esp8266/umm_malloc/umm_malloc_cfg.h | 58 ++++----- cores/esp8266/umm_malloc/umm_malloc_cfgport.h | 116 ++++++++++++++++++ 6 files changed, 199 insertions(+), 95 deletions(-) diff --git a/cores/esp8266/Esp.cpp b/cores/esp8266/Esp.cpp index 400bb10800..9bc30b790b 100644 --- a/cores/esp8266/Esp.cpp +++ b/cores/esp8266/Esp.cpp @@ -219,7 +219,7 @@ uint16_t EspClass::getVcc(void) uint32_t EspClass::getFreeHeap(void) { - return umm_free_heap_size(); + return umm_free_heap_size_lw(); } uint32_t EspClass::getMaxFreeBlockSize(void) diff --git a/cores/esp8266/umm_malloc/Notes.h b/cores/esp8266/umm_malloc/Notes.h index 2cd2d88fa6..af0852b430 100644 --- a/cores/esp8266/umm_malloc/Notes.h +++ b/cores/esp8266/umm_malloc/Notes.h @@ -319,28 +319,26 @@ Enhancement ideas: upstream umm_malloc's `umm_free_heap_size()` and require the build option UMM_INLINE_METRICS. Otherwise, fail the build. - Use function name `umm_free_heap_size()` to support external request for - current heap size. + Use function name `umm_free_heap_size_lw()` to support external request for + current heap size. When build options result in fallback using umm_info.c, + ensure UMM_INLINE_METRICS enabled and alias to `umm_free_heap_size()`. - For the multiple Heap case `xPortGetFreeHeapSize()` becomes a unique function + For the multiple Heap case, `xPortGetFreeHeapSize()` becomes a unique function and reports only DRAM free heap size. Now `system_get_free_heap_size()` will always report DRAM free Heap size. This might be a breaking change. Specifics: - * rename `umm_free_heap_size_lw()` to `umm_free_heap_size()` for builds that - include option UMM_STATS(default) or UMM_STATS_FULL. - * rename upstream umm_malloc's `umm_free_heap_size()` to - `umm_free_heap_size_info()`. + * Support `umm_free_heap_size_lw()` as an `extern`. * When the build options UMM_STATS/UMM_STATS_FULL are not used, fallback to - the upstream umm_malloc's original `umm_free_heap_size()` function in - umm_info.c + the upstream umm_malloc's `umm_free_heap_size()` function in umm_info.c * require the UMM_INLINE_METRICS build option. + * assign `umm_free_heap_size_lw()` as an alias to `umm_free_heap_size()` * `xPortGetFreeHeapSize()` - * For single heap builds, alias to `umm_free_heap_size()` - * For multi-heap builds, add a dedicated function that always reports + * For single heap builds, alias to `umm_free_heap_size_lw()` + * For multiple Heaps builds, add a dedicated function that always reports DRAM results. */ diff --git a/cores/esp8266/umm_malloc/umm_info.c b/cores/esp8266/umm_malloc/umm_info.c index d395fc5c29..c0ebb7948f 100644 --- a/cores/esp8266/umm_malloc/umm_info.c +++ b/cores/esp8266/umm_malloc/umm_info.c @@ -175,37 +175,14 @@ size_t umm_free_heap_size_core(umm_heap_context_t *_context) { } /* - Need to expose a function for getting the current free heap size - Using umm_free_heap_size for this purpose. - - For an expanded discussion see Notes.h, entry dated "Sep 26, 2022" -*/ -#if defined(UMM_STATS) || defined(UMM_STATS_FULL) -/* - For this build option, see umm_free_heap_size in umm_local.c -*/ - -/* - Make upstream logic available under a new name. - May be useful for sanity checks between methods. - Only takes .bin space if used. + When used as the fallback option for supporting exported function + `umm_free_heap_size_lw()`, the build option UMM_INLINE_METRICS is required. + Otherwise, umm_info() would be used to complete the operation, which uses a + time-consuming method for getting free Heap and runs with interrupts off, + which can negatively impact WiFi operations. Also, it cannot support calls + from ISRs, `umm_info()` runs from flash. */ -size_t umm_free_heap_size_info(void) - -#else -/* - Not our default build path. For this path to function well with WiFi enabled, - you must use the build option UMM_INLINE_METRICS. Otherwise, umm_info() is - used to complete the operation, which uses a time-consuming method for getting - free Heap and runs with interrupts off. Also, it cannot support calls from - ISRs, `umm_info()` runs from flash. - - Current build test in umm_local.c fails the build option that uses the - umm_info() method. -*/ -size_t umm_free_heap_size(void) -#endif -{ +size_t umm_free_heap_size(void) { #ifndef UMM_INLINE_METRICS umm_info(NULL, false); #endif diff --git a/cores/esp8266/umm_malloc/umm_local.c b/cores/esp8266/umm_malloc/umm_local.c index c842214e80..bdb741e08a 100644 --- a/cores/esp8266/umm_malloc/umm_local.c +++ b/cores/esp8266/umm_malloc/umm_local.c @@ -161,16 +161,29 @@ void umm_poison_free_fl(void *ptr, const char *file, int line) { /* ------------------------------------------------------------------------ */ #if defined(UMM_STATS) || defined(UMM_STATS_FULL) || defined(UMM_INFO) +/* + For internal, mainly used by UMM_STATS_FULL; exported so external components + can perform Heap related calculations. +*/ size_t umm_block_size(void) { return sizeof(umm_block); } #endif +/* + Need to expose a function to support getting the current free heap size. + Export `size_t umm_free_heap_size_lw(void)` for this purpose. + Used by ESP.getFreeHeap(). + For an expanded discussion see Notes.h, entry dated "Sep 26, 2022" +*/ #if defined(UMM_STATS) || defined(UMM_STATS_FULL) -// Support a consistant external name across build options. Used by -// ESP.getFreeHeap(). Keep complete call path in IRAM. -size_t umm_free_heap_size(void) { +/* + Default build option to support export. + + Keep complete call path in IRAM. +*/ +size_t umm_free_heap_size_lw(void) { UMM_CHECK_INITIALIZED(); umm_heap_context_t *_context = umm_get_current_heap(); @@ -179,41 +192,57 @@ size_t umm_free_heap_size(void) { #elif defined(UMM_INLINE_METRICS) /* - Use `size_t umm_free_heap_size(void)` (in umm_info.c) from upstream - umm_malloc. It must have the UMM_INLINE_METRICS Build option enabled to - support free heap size reporting without the use of `umm_info()` + For the fallback option using `size_t umm_free_heap_size(void)`, we must have + the UMM_INLINE_METRICS build option enabled to support free heap size + reporting without the use of `umm_info()`. */ +size_t umm_free_heap_size_lw(void) __attribute__ ((alias("umm_free_heap_size"))); + #else -// We require a resources to track and report free heap with low overhead. -// See umm_free_heap_size in umm_info.c for more details +/* + We require a resource to track and report free Heap size with low overhead. + For an expanded discussion see Notes.h, entry dated "Sep 26, 2022" +*/ #error UMM_INLINE_METRICS, UMM_STATS, or UMM_STATS_FULL needs to be defined. #endif +#if defined(UMM_STATS) || defined(UMM_STATS_FULL) +size_t umm_free_heap_size_core_lw(umm_heap_context_t *_context) { + return (size_t)_context->UMM_FREE_BLOCKS * sizeof(umm_block); +} + +#elif defined(UMM_INFO) +// Backfill support for umm_free_heap_size_core_lw() +size_t umm_free_heap_size_core_lw(umm_heap_context_t *_context) __attribute__ ((alias("umm_free_heap_size_core"))); +#endif + /* - This API is called by `system_get_free_heap_size()` which is in IRAM. Use IRAM - to ensure the call chain is in IRAM. Which implies it may be called from an - ISR. + This API is called by `system_get_free_heap_size()` which is in IRAM. Driving + the assumption the callee may be in an ISR or Cache_Read_Disable state. Use + IRAM to ensure that the complete call chain is in IRAM. To satisfy this requirement, we need UMM_STATS... or UMM_INLINE_METRICS defined. These support an always available without intense computation free-Heap value. Like the other vPort... APIs used by the SDK, this must always report on the - DRAM Heap not current Heap. + DRAM Heap not the current Heap. */ #if (UMM_NUM_HEAPS == 1) // Reduce IRAM usage for the single Heap case +#if defined(UMM_STATS) || defined(UMM_STATS_FULL) +size_t xPortGetFreeHeapSize(void) __attribute__ ((alias("umm_free_heap_size_lw"))); +#else size_t xPortGetFreeHeapSize(void) __attribute__ ((alias("umm_free_heap_size"))); +#endif #else size_t xPortGetFreeHeapSize(void) { + #if defined(UMM_STATS) || defined(UMM_STATS_FULL) || defined(UMM_INLINE_METRICS) UMM_CHECK_INITIALIZED(); umm_heap_context_t *_context = umm_get_heap_by_id(UMM_HEAP_DRAM); - #if defined(UMM_STATS) || defined(UMM_STATS_FULL) - return (size_t)_context->UMM_FREE_BLOCKS * sizeof(umm_block); - #elif defined(UMM_INLINE_METRICS) - return umm_free_heap_size_core(_context); + return umm_free_heap_size_core_lw(_context); #else // At this time, this build path is not reachable. In case things change, // keep build check. diff --git a/cores/esp8266/umm_malloc/umm_malloc_cfg.h b/cores/esp8266/umm_malloc/umm_malloc_cfg.h index 9b756faa08..24c5e5865b 100644 --- a/cores/esp8266/umm_malloc/umm_malloc_cfg.h +++ b/cores/esp8266/umm_malloc/umm_malloc_cfg.h @@ -106,31 +106,6 @@ extern "C" { #include "umm_malloc_cfgport.h" #endif -#define UMM_BEST_FIT -#define UMM_INFO -// #define UMM_INLINE_METRICS -#define UMM_STATS - -/* - * To support API call, system_show_malloc(), -DUMM_INFO is required. - * - * For the ESP8266 we need an ISR safe function to call for implementing - * xPortGetFreeHeapSize(). We can get this with one of these options: - * 1) -DUMM_STATS or -DUMM_STATS_FULL - * 2) -DUMM_INLINE_METRICS (and implicitly includes -DUMM_INFO) - * - * If frequent calls are made to ESP.getHeapFragmentation(), - * -DUMM_INLINE_METRICS would reduce long periods of interrupts disabled caused - * by frequent calls to `umm_info()`. Instead, the computations get distributed - * across each malloc, realloc, and free. This appears to require an additional - * 116 bytes of IRAM vs using `UMM_STATS` with `UMM_INFO`. - * - * When both UMM_STATS and UMM_INLINE_METRICS are defined, macros and structures - * have been optimized to reduce duplications. - * - */ - - /* A couple of macros to make packing structures less compiler dependent */ #define UMM_H_ATTPACKPRE @@ -177,12 +152,22 @@ extern "C" { #define UMM_FRAGMENTATION_METRIC_REMOVE(c) #endif // UMM_INLINE_METRICS +struct UMM_HEAP_CONTEXT; +typedef struct UMM_HEAP_CONTEXT umm_heap_context_t; + +/* + Must always be defined. Core support for getting free Heap size. + When possible, access via ESP.getFreeHeap(); +*/ +extern size_t umm_free_heap_size_lw(void); +extern size_t umm_free_heap_size_core_lw(umm_heap_context_t *_context); + /* -------------------------------------------------------------------------- */ /* * -D UMM_INFO : * - * Enables a dup of the heap contents and a function to return the total + * Enables a dump of the heap contents and a function to return the total * heap size that is unallocated - note this is not the same as the largest * unallocated block on the heap! */ @@ -209,20 +194,20 @@ typedef struct UMM_HEAP_INFO_t { UMM_HEAP_INFO; // extern UMM_HEAP_INFO ummHeapInfo; -struct UMM_HEAP_CONTEXT; -typedef struct UMM_HEAP_CONTEXT umm_heap_context_t; - extern ICACHE_FLASH_ATTR void *umm_info(void *ptr, bool force); -#if (defined(UMM_STATS) || defined(UMM_STATS_FULL)) && defined(UMM_INFO) -extern ICACHE_FLASH_ATTR size_t umm_free_heap_size_info(void); -#endif +#if defined(UMM_STATS) || defined(UMM_STATS_FULL) +extern ICACHE_FLASH_ATTR size_t umm_free_heap_size(void); +extern ICACHE_FLASH_ATTR size_t umm_free_heap_size_core(umm_heap_context_t *_context); +#else extern size_t umm_free_heap_size(void); +extern size_t umm_free_heap_size_core(umm_heap_context_t *_context); +#endif + // umm_max_block_size changed to umm_max_free_block_size in upstream. extern ICACHE_FLASH_ATTR size_t umm_max_block_size(void); extern ICACHE_FLASH_ATTR int umm_usage_metric(void); extern ICACHE_FLASH_ATTR int umm_fragmentation_metric(void); -extern ICACHE_FLASH_ATTR size_t umm_free_heap_size_core(umm_heap_context_t *_context); extern ICACHE_FLASH_ATTR size_t umm_max_block_size_core(umm_heap_context_t *_context); extern ICACHE_FLASH_ATTR int umm_usage_metric_core(umm_heap_context_t *_context); extern ICACHE_FLASH_ATTR int umm_fragmentation_metric_core(umm_heap_context_t *_context); @@ -232,9 +217,9 @@ extern ICACHE_FLASH_ATTR int umm_fragmentation_metric_core(umm_heap_context_t *_ #define umm_max_block_size() (0) #define umm_fragmentation_metric() (0) #define umm_usage_metric() (0) - #define umm_free_heap_size_core() (0) - #define umm_max_block_size_core() (0) - #define umm_fragmentation_metric_core() (0) + #define umm_free_heap_size_core(c) (0) + #define umm_max_block_size_core(c) (0) + #define umm_fragmentation_metric_core(c) (0) #endif /* @@ -305,7 +290,6 @@ UMM_STATISTICS; #define STATS__OOM_UPDATE() _context->UMM_OOM_COUNT += 1 -extern size_t umm_free_heap_size_lw(void); extern size_t umm_get_oom_count(void); #else // not UMM_STATS or UMM_STATS_FULL diff --git a/cores/esp8266/umm_malloc/umm_malloc_cfgport.h b/cores/esp8266/umm_malloc/umm_malloc_cfgport.h index 02db6fc66c..7ee71d3819 100644 --- a/cores/esp8266/umm_malloc/umm_malloc_cfgport.h +++ b/cores/esp8266/umm_malloc/umm_malloc_cfgport.h @@ -19,6 +19,12 @@ #include "c_types.h" +/* + * Between UMM_BEST_FIT or UMM_FIRST_FIT, UMM_BEST_FIT is the better option for + * reducing heap fragmentation. With no selection made, UMM_BEST_FIT is used. + * See umm_malloc_cfg.h for more information. + */ + /* * -DUMM_INIT_USE_IRAM * @@ -87,5 +93,115 @@ extern char _heap_start[]; #define UMM_HEAP_STACK_DEPTH 32 #endif +/* + * To support API call, system_show_malloc(), -DUMM_INFO is required. + * + * UMM_INFO is needed to support several EspClass methods and umm_info(). + * Partial EspClass method list: + * uint32_t EspClass::getMaxFreeBlockSize() + * void EspClass::getHeapStats(uint32_t* hfree, uint32_t* hmax, uint8_t* hfrag) + * uint8_t EspClass::getHeapFragmentation() + * + * For the ESP8266 we need an ISR safe function to call for implementing + * xPortGetFreeHeapSize(). We can get this with one of these options: + * 1) -DUMM_STATS or -DUMM_STATS_FULL + * 2) -DUMM_INLINE_METRICS (and implicitly includes -DUMM_INFO) + * + * If frequent calls are made to ESP.getHeapFragmentation(), + * -DUMM_INLINE_METRICS would reduce long periods of interrupts disabled caused + * by frequent calls to `umm_info()`. Instead, the computations get distributed + * across each malloc, realloc, and free. This appears to require an additional + * 116 bytes of IRAM vs using `UMM_STATS` with `UMM_INFO`. + * + * When both UMM_STATS and UMM_INLINE_METRICS are defined, macros and structures + * have been optimized to reduce duplications. + * + * You can use just UMM_INFO and drop UMM_STATS/UMM_STATS_FULL gaining back + * some IROM at the expense of IRAM. + * + * If you don't require the methods in EspClass that are dependent on UMM_INFO, + * you can use just UMM_STATS and save on IROM and a little IRAM. + */ +#if defined(UMM_STATS) || defined(UMM_STATS_FULL) || defined(UMM_INLINE_METRICS) || defined(UMM_INFO) +/* + User defined via build options eg. Sketch.ino.globals.h +*/ +#else +/* + Set expected/implicit defaults for complete support of EspClass methods. +*/ +#define UMM_INFO 1 +#define UMM_STATS 1 +#endif + +/* + For `-Dname`, gcc assigns a value of 1 and this works find; however, + if `-Dname=0` is used, the intended results will not be obtained. + + Make value and valueless defines compliant with their usage in umm_malloc: + `#define name` => #define name 1 + `#define name 0` => #undef name +*/ +#if ((1 - UMM_BEST_FIT - 1) == 2) +// Assume 1 for define w/o value +#undef UMM_BEST_FIT +#define UMM_BEST_FIT 1 +#elif ((1 - UMM_BEST_FIT - 1) == 0) +#undef UMM_BEST_FIT +#endif +#if ((1 - UMM_FIRST_FIT - 1) == 2) +#undef UMM_FIRST_FIT +#define UMM_FIRST_FIT 1 +#elif ((1 - UMM_FIRST_FIT - 1) == 0) +#undef UMM_FIRST_FIT +#endif + +#if ((1 - UMM_INFO - 1) == 2) +#undef UMM_INFO +#define UMM_INFO 1 +#elif ((1 - UMM_INFO - 1) == 0) +#undef UMM_INFO +#endif +#if ((1 - UMM_INLINE_METRICS - 1) == 2) +#undef UMM_INLINE_METRICS +#define UMM_INLINE_METRICS 1 +#elif ((1 - UMM_INLINE_METRICS - 1) == 0) +#undef UMM_INLINE_METRICS +#endif + +#if ((1 - UMM_STATS - 1) == 2) +#undef UMM_STATS +#define UMM_STATS 1 +#elif ((1 - UMM_STATS - 1) == 0) +#undef UMM_STATS +#endif +#if ((1 - UMM_STATS_FULL - 1) == 2) +#undef UMM_STATS_FULL +#define UMM_STATS_FULL 1 +#elif ((1 - UMM_STATS_FULL - 1) == 0) +#undef UMM_STATS_FULL +#endif + + +#if defined(UMM_INLINE_METRICS) +// Dependent on UMM_INFO if missing enable. +#ifndef UMM_INFO +#define UMM_INFO 1 +#endif +#endif + +#if defined(UMM_STATS) || defined(UMM_STATS_FULL) +// We have support for free Heap size +#if defined(UMM_STATS) && defined(UMM_STATS_FULL) +#error "Build option conflict, specify either UMM_STATS or UMM_STATS_FULL." +#endif +#elif defined(UMM_INFO) +// ensure fallback support for free Heap size +#ifndef UMM_INLINE_METRICS +#define UMM_INLINE_METRICS 1 +#endif +#else +#error "Specify at least one of these build options: (UMM_STATS or UMM_STATS_FULL) and/or UMM_INFO and/or UMM_INLINE_METRICS" +#endif #endif From 9f700b9f9c4ed525771e8fc7deb9946852acbab9 Mon Sep 17 00:00:00 2001 From: M Hightower <27247790+mhightower83@users.noreply.github.com> Date: Sun, 2 Oct 2022 14:24:46 -0700 Subject: [PATCH 3/6] improve comments --- cores/esp8266/umm_malloc/umm_local.c | 2 +- cores/esp8266/umm_malloc/umm_malloc_cfgport.h | 43 ++++++++++--------- 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/cores/esp8266/umm_malloc/umm_local.c b/cores/esp8266/umm_malloc/umm_local.c index bdb741e08a..b02e511cea 100644 --- a/cores/esp8266/umm_malloc/umm_local.c +++ b/cores/esp8266/umm_malloc/umm_local.c @@ -246,7 +246,7 @@ size_t xPortGetFreeHeapSize(void) { #else // At this time, this build path is not reachable. In case things change, // keep build check. - // Not in IRAM umm_info() has to be used to complete this operation. + // Not in IRAM, umm_info() would have been used to complete this operation. #error "No ISR safe function available to implement xPortGetFreeHeapSize()" #endif } diff --git a/cores/esp8266/umm_malloc/umm_malloc_cfgport.h b/cores/esp8266/umm_malloc/umm_malloc_cfgport.h index 7ee71d3819..44c8164034 100644 --- a/cores/esp8266/umm_malloc/umm_malloc_cfgport.h +++ b/cores/esp8266/umm_malloc/umm_malloc_cfgport.h @@ -94,33 +94,36 @@ extern char _heap_start[]; #endif /* - * To support API call, system_show_malloc(), -DUMM_INFO is required. + * The NONOS SDK API requires function `umm_info()` for implementing + * `system_show_malloc()`. Build option `-DUMM_INFO` enables this support. * - * UMM_INFO is needed to support several EspClass methods and umm_info(). + * Also, `-DUMM_INFO` is needed to support several EspClass methods. * Partial EspClass method list: - * uint32_t EspClass::getMaxFreeBlockSize() - * void EspClass::getHeapStats(uint32_t* hfree, uint32_t* hmax, uint8_t* hfrag) - * uint8_t EspClass::getHeapFragmentation() + * `uint32_t EspClass::getMaxFreeBlockSize()` + * `void EspClass::getHeapStats(uint32_t* hfree, uint32_t* hmax, uint8_t* hfrag)` + * `uint8_t EspClass::getHeapFragmentation()` * - * For the ESP8266 we need an ISR safe function to call for implementing - * xPortGetFreeHeapSize(). We can get this with one of these options: - * 1) -DUMM_STATS or -DUMM_STATS_FULL - * 2) -DUMM_INLINE_METRICS (and implicitly includes -DUMM_INFO) + * The NONOS SDK API requires an ISR safe function to call for implementing + * `xPortGetFreeHeapSize()`. Use one of these options: + * 1) `-DUMM_STATS` or `-DUMM_STATS_FULL` + * 2) `-DUMM_INLINE_METRICS` (implicitly includes `-DUMM_INFO`) * - * If frequent calls are made to ESP.getHeapFragmentation(), - * -DUMM_INLINE_METRICS would reduce long periods of interrupts disabled caused - * by frequent calls to `umm_info()`. Instead, the computations get distributed - * across each malloc, realloc, and free. This appears to require an additional - * 116 bytes of IRAM vs using `UMM_STATS` with `UMM_INFO`. + * If frequent calls are made to `ESP.getHeapFragmentation()`, using build + * option `-DUMM_INLINE_METRICS` would reduce long periods of interrupts + * disabled caused by frequent calls to `umm_info().` Instead, the computations + * get distributed across each malloc, realloc, and free. Requires approximately + * 116 more bytes of IRAM when compared to the build option `-DUMM_STATS` with + * `-DUMM_INFO.` * - * When both UMM_STATS and UMM_INLINE_METRICS are defined, macros and structures - * have been optimized to reduce duplications. + * When both `-DUMM_STATS` and `-DUMM_INLINE_METRICS` are defined, macros and + * structures are optimized to reduce duplications. * - * You can use just UMM_INFO and drop UMM_STATS/UMM_STATS_FULL gaining back - * some IROM at the expense of IRAM. + * You can use `-DUMM_INFO` with `-DUMM_INLINE_METRICS` and drop + * `-DUMM_STATS(_FULL)` gaining back some IROM at the expense of IRAM. * - * If you don't require the methods in EspClass that are dependent on UMM_INFO, - * you can use just UMM_STATS and save on IROM and a little IRAM. + * If you don't require the methods in EspClass that are dependent on functions + * from the `-DUMM_INFO` build option, you can use only `-DUMM_STATS` and save + * on IROM and a little IRAM. */ #if defined(UMM_STATS) || defined(UMM_STATS_FULL) || defined(UMM_INLINE_METRICS) || defined(UMM_INFO) /* From 9612e5f395ca16a32bce98d2691327e8ea1e44b9 Mon Sep 17 00:00:00 2001 From: M Hightower <27247790+mhightower83@users.noreply.github.com> Date: Mon, 3 Oct 2022 14:06:38 -0700 Subject: [PATCH 4/6] improve comments Improve empty function situation around build option UMM_INFO. --- cores/esp8266/Esp-frag.cpp | 2 ++ cores/esp8266/Esp.cpp | 2 ++ cores/esp8266/Esp.h | 8 +++++++- cores/esp8266/heap.cpp | 4 +++- cores/esp8266/umm_malloc/umm_info.c | 6 ++++++ cores/esp8266/umm_malloc/umm_malloc_cfg.h | 4 ++-- cores/esp8266/umm_malloc/umm_malloc_cfgport.h | 10 ++++++++-- 7 files changed, 30 insertions(+), 6 deletions(-) diff --git a/cores/esp8266/Esp-frag.cpp b/cores/esp8266/Esp-frag.cpp index 6913179974..a5e18bd666 100644 --- a/cores/esp8266/Esp-frag.cpp +++ b/cores/esp8266/Esp-frag.cpp @@ -23,6 +23,7 @@ #include "coredecls.h" #include "Esp.h" +#if defined(UMM_INFO) || defined(UMM_INFO_EMPTY) void EspClass::getHeapStats(uint32_t* hfree, uint32_t* hmax, uint8_t* hfrag) { // L2 / Euclidean norm of free block sizes. @@ -60,3 +61,4 @@ uint8_t EspClass::getHeapFragmentation() { return (uint8_t)umm_fragmentation_metric(); } +#endif diff --git a/cores/esp8266/Esp.cpp b/cores/esp8266/Esp.cpp index 9bc30b790b..4fb270107e 100644 --- a/cores/esp8266/Esp.cpp +++ b/cores/esp8266/Esp.cpp @@ -222,10 +222,12 @@ uint32_t EspClass::getFreeHeap(void) return umm_free_heap_size_lw(); } +#if defined(UMM_INFO) || defined(UMM_INFO_EMPTY) uint32_t EspClass::getMaxFreeBlockSize(void) { return umm_max_block_size(); } +#endif uint32_t EspClass::getFreeContStack() { diff --git a/cores/esp8266/Esp.h b/cores/esp8266/Esp.h index 1bb793ef7f..c65c3ca152 100644 --- a/cores/esp8266/Esp.h +++ b/cores/esp8266/Esp.h @@ -114,11 +114,17 @@ class EspClass { static uint32_t getChipId(); static uint32_t getFreeHeap(); +#if defined(UMM_INFO) static uint32_t getMaxFreeBlockSize(); static uint8_t getHeapFragmentation(); // in % static void getHeapStats(uint32_t* free = nullptr, uint16_t* max = nullptr, uint8_t* frag = nullptr) __attribute__((deprecated("Use 'uint32_t*' on max, 2nd argument"))); static void getHeapStats(uint32_t* free = nullptr, uint32_t* max = nullptr, uint8_t* frag = nullptr); - +#elif defined(UMM_INFO_EMPTY) + static uint32_t getMaxFreeBlockSize() __attribute__((deprecated("This method is empty use the build option UMM_INFO to complete."))); + static uint8_t getHeapFragmentation() __attribute__((deprecated("This method is empty use the build option UMM_INFO to complete."))); + static void getHeapStats(uint32_t* free = nullptr, uint16_t* max = nullptr, uint8_t* frag = nullptr) __attribute__((deprecated("This method is empty use the build option UMM_INFO to complete."))); + static void getHeapStats(uint32_t* free = nullptr, uint32_t* max = nullptr, uint8_t* frag = nullptr) __attribute__((deprecated("This method is empty use the build option UMM_INFO to complete."))); +#endif static uint32_t getFreeContStack(); static void resetFreeContStack(); diff --git a/cores/esp8266/heap.cpp b/cores/esp8266/heap.cpp index 78011ed248..131973ae9f 100644 --- a/cores/esp8266/heap.cpp +++ b/cores/esp8266/heap.cpp @@ -11,7 +11,7 @@ extern "C" size_t umm_umul_sat(const size_t a, const size_t b); extern "C" void z2EapFree(void *ptr, const char* file, int line) __attribute__((weak, alias("vPortFree"), nothrow)); // I don't understand all the compiler noise around this alias. // Adding "__attribute__ ((nothrow))" seems to resolve the issue. -// This may be relevant: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81824 +// This may be relevant: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81824 // Need FORCE_ALWAYS_INLINE to put HeapSelect class constructor/deconstructor in IRAM #define FORCE_ALWAYS_INLINE_HEAP_SELECT @@ -342,8 +342,10 @@ size_t IRAM_ATTR xPortWantedSizeAlign(size_t size) void system_show_malloc(void) { +#ifdef UMM_INFO HeapSelectDram ephemeral; umm_info(NULL, true); +#endif } /* diff --git a/cores/esp8266/umm_malloc/umm_info.c b/cores/esp8266/umm_malloc/umm_info.c index c0ebb7948f..8c21f104fb 100644 --- a/cores/esp8266/umm_malloc/umm_info.c +++ b/cores/esp8266/umm_malloc/umm_info.c @@ -269,6 +269,12 @@ static void umm_fragmentation_metric_remove(umm_heap_context_t *_context, uint16 #endif // UMM_INLINE_METRICS /* ------------------------------------------------------------------------ */ +#elif defined(UMM_INFO_EMPTY) +void *umm_info(void *ptr, bool force) { + (void)ptr; + (void)force; + return NULL; +} #endif #endif // defined(BUILD_UMM_MALLOC_C) diff --git a/cores/esp8266/umm_malloc/umm_malloc_cfg.h b/cores/esp8266/umm_malloc/umm_malloc_cfg.h index 24c5e5865b..28222cd861 100644 --- a/cores/esp8266/umm_malloc/umm_malloc_cfg.h +++ b/cores/esp8266/umm_malloc/umm_malloc_cfg.h @@ -211,8 +211,8 @@ extern ICACHE_FLASH_ATTR int umm_fragmentation_metric(void); extern ICACHE_FLASH_ATTR size_t umm_max_block_size_core(umm_heap_context_t *_context); extern ICACHE_FLASH_ATTR int umm_usage_metric_core(umm_heap_context_t *_context); extern ICACHE_FLASH_ATTR int umm_fragmentation_metric_core(umm_heap_context_t *_context); -#else - #define umm_info(p,b) +#elif defined(UMM_INFO_EMPTY) + extern ICACHE_FLASH_ATTR void *umm_info(void *ptr, bool force) __attribute__((deprecated("This method is empty use the build option UMM_INFO to complete."))); #define umm_free_heap_size() (0) #define umm_max_block_size() (0) #define umm_fragmentation_metric() (0) diff --git a/cores/esp8266/umm_malloc/umm_malloc_cfgport.h b/cores/esp8266/umm_malloc/umm_malloc_cfgport.h index 44c8164034..3a7cafd7c4 100644 --- a/cores/esp8266/umm_malloc/umm_malloc_cfgport.h +++ b/cores/esp8266/umm_malloc/umm_malloc_cfgport.h @@ -124,6 +124,11 @@ extern char _heap_start[]; * If you don't require the methods in EspClass that are dependent on functions * from the `-DUMM_INFO` build option, you can use only `-DUMM_STATS` and save * on IROM and a little IRAM. + * + * Build option `-DUMM_INFO_EMPTY` ignored when `-DUMM_INFO` enabled. + * `-DUMM_INFO_EMPTY` provides empty versions of the functions supplied by + * `UMM_INFO` intended for easing build option experimentation. It will report + * noisy depreciation messages until you clean up all references. */ #if defined(UMM_STATS) || defined(UMM_STATS_FULL) || defined(UMM_INLINE_METRICS) || defined(UMM_INFO) /* @@ -138,7 +143,7 @@ extern char _heap_start[]; #endif /* - For `-Dname`, gcc assigns a value of 1 and this works find; however, + For `-Dname`, gcc assigns a value of 1 and this works fine; however, if `-Dname=0` is used, the intended results will not be obtained. Make value and valueless defines compliant with their usage in umm_malloc: @@ -146,7 +151,8 @@ extern char _heap_start[]; `#define name 0` => #undef name */ #if ((1 - UMM_BEST_FIT - 1) == 2) -// Assume 1 for define w/o value +// When UMM_BEST_FIT is defined w/o value, the computation becomes +// (1 - - 1) == 2 => (1 + 1) == 2 #undef UMM_BEST_FIT #define UMM_BEST_FIT 1 #elif ((1 - UMM_BEST_FIT - 1) == 0) From 3f958fb7525d601bd0e51de314f4e74c738b7831 Mon Sep 17 00:00:00 2001 From: M Hightower <27247790+mhightower83@users.noreply.github.com> Date: Tue, 4 Oct 2022 13:00:56 -0700 Subject: [PATCH 5/6] Removed empty defines related to UMM_INFO --- cores/esp8266/Esp-frag.cpp | 2 +- cores/esp8266/Esp.cpp | 2 +- cores/esp8266/Esp.h | 6 +----- cores/esp8266/umm_malloc/umm_info.c | 6 ------ cores/esp8266/umm_malloc/umm_malloc_cfg.h | 9 --------- cores/esp8266/umm_malloc/umm_malloc_cfgport.h | 4 ---- 6 files changed, 3 insertions(+), 26 deletions(-) diff --git a/cores/esp8266/Esp-frag.cpp b/cores/esp8266/Esp-frag.cpp index a5e18bd666..249ff5770c 100644 --- a/cores/esp8266/Esp-frag.cpp +++ b/cores/esp8266/Esp-frag.cpp @@ -23,7 +23,7 @@ #include "coredecls.h" #include "Esp.h" -#if defined(UMM_INFO) || defined(UMM_INFO_EMPTY) +#if defined(UMM_INFO) void EspClass::getHeapStats(uint32_t* hfree, uint32_t* hmax, uint8_t* hfrag) { // L2 / Euclidean norm of free block sizes. diff --git a/cores/esp8266/Esp.cpp b/cores/esp8266/Esp.cpp index 4fb270107e..0109bbe943 100644 --- a/cores/esp8266/Esp.cpp +++ b/cores/esp8266/Esp.cpp @@ -222,7 +222,7 @@ uint32_t EspClass::getFreeHeap(void) return umm_free_heap_size_lw(); } -#if defined(UMM_INFO) || defined(UMM_INFO_EMPTY) +#if defined(UMM_INFO) uint32_t EspClass::getMaxFreeBlockSize(void) { return umm_max_block_size(); diff --git a/cores/esp8266/Esp.h b/cores/esp8266/Esp.h index c65c3ca152..b852890687 100644 --- a/cores/esp8266/Esp.h +++ b/cores/esp8266/Esp.h @@ -22,6 +22,7 @@ #define ESP_H #include +#include "umm_malloc/umm_malloc_cfg.h" #include "core_esp8266_features.h" #include "spi_vendors.h" @@ -119,11 +120,6 @@ class EspClass { static uint8_t getHeapFragmentation(); // in % static void getHeapStats(uint32_t* free = nullptr, uint16_t* max = nullptr, uint8_t* frag = nullptr) __attribute__((deprecated("Use 'uint32_t*' on max, 2nd argument"))); static void getHeapStats(uint32_t* free = nullptr, uint32_t* max = nullptr, uint8_t* frag = nullptr); -#elif defined(UMM_INFO_EMPTY) - static uint32_t getMaxFreeBlockSize() __attribute__((deprecated("This method is empty use the build option UMM_INFO to complete."))); - static uint8_t getHeapFragmentation() __attribute__((deprecated("This method is empty use the build option UMM_INFO to complete."))); - static void getHeapStats(uint32_t* free = nullptr, uint16_t* max = nullptr, uint8_t* frag = nullptr) __attribute__((deprecated("This method is empty use the build option UMM_INFO to complete."))); - static void getHeapStats(uint32_t* free = nullptr, uint32_t* max = nullptr, uint8_t* frag = nullptr) __attribute__((deprecated("This method is empty use the build option UMM_INFO to complete."))); #endif static uint32_t getFreeContStack(); static void resetFreeContStack(); diff --git a/cores/esp8266/umm_malloc/umm_info.c b/cores/esp8266/umm_malloc/umm_info.c index 8c21f104fb..c0ebb7948f 100644 --- a/cores/esp8266/umm_malloc/umm_info.c +++ b/cores/esp8266/umm_malloc/umm_info.c @@ -269,12 +269,6 @@ static void umm_fragmentation_metric_remove(umm_heap_context_t *_context, uint16 #endif // UMM_INLINE_METRICS /* ------------------------------------------------------------------------ */ -#elif defined(UMM_INFO_EMPTY) -void *umm_info(void *ptr, bool force) { - (void)ptr; - (void)force; - return NULL; -} #endif #endif // defined(BUILD_UMM_MALLOC_C) diff --git a/cores/esp8266/umm_malloc/umm_malloc_cfg.h b/cores/esp8266/umm_malloc/umm_malloc_cfg.h index 28222cd861..706f0b7363 100644 --- a/cores/esp8266/umm_malloc/umm_malloc_cfg.h +++ b/cores/esp8266/umm_malloc/umm_malloc_cfg.h @@ -211,15 +211,6 @@ extern ICACHE_FLASH_ATTR int umm_fragmentation_metric(void); extern ICACHE_FLASH_ATTR size_t umm_max_block_size_core(umm_heap_context_t *_context); extern ICACHE_FLASH_ATTR int umm_usage_metric_core(umm_heap_context_t *_context); extern ICACHE_FLASH_ATTR int umm_fragmentation_metric_core(umm_heap_context_t *_context); -#elif defined(UMM_INFO_EMPTY) - extern ICACHE_FLASH_ATTR void *umm_info(void *ptr, bool force) __attribute__((deprecated("This method is empty use the build option UMM_INFO to complete."))); - #define umm_free_heap_size() (0) - #define umm_max_block_size() (0) - #define umm_fragmentation_metric() (0) - #define umm_usage_metric() (0) - #define umm_free_heap_size_core(c) (0) - #define umm_max_block_size_core(c) (0) - #define umm_fragmentation_metric_core(c) (0) #endif /* diff --git a/cores/esp8266/umm_malloc/umm_malloc_cfgport.h b/cores/esp8266/umm_malloc/umm_malloc_cfgport.h index 3a7cafd7c4..8b463dea96 100644 --- a/cores/esp8266/umm_malloc/umm_malloc_cfgport.h +++ b/cores/esp8266/umm_malloc/umm_malloc_cfgport.h @@ -125,10 +125,6 @@ extern char _heap_start[]; * from the `-DUMM_INFO` build option, you can use only `-DUMM_STATS` and save * on IROM and a little IRAM. * - * Build option `-DUMM_INFO_EMPTY` ignored when `-DUMM_INFO` enabled. - * `-DUMM_INFO_EMPTY` provides empty versions of the functions supplied by - * `UMM_INFO` intended for easing build option experimentation. It will report - * noisy depreciation messages until you clean up all references. */ #if defined(UMM_STATS) || defined(UMM_STATS_FULL) || defined(UMM_INLINE_METRICS) || defined(UMM_INFO) /* From b7fb798a8dd738a5b218613a6a97ee64e13699f6 Mon Sep 17 00:00:00 2001 From: M Hightower <27247790+mhightower83@users.noreply.github.com> Date: Wed, 5 Oct 2022 11:35:46 -0700 Subject: [PATCH 6/6] untangle some includes and moved some around --- cores/esp8266/Arduino.h | 8 +- cores/esp8266/Esp-frag.cpp | 1 - cores/esp8266/Esp.h | 1 - cores/esp8266/heap_api_debug.h | 74 +++++++++++++++++ cores/esp8266/mmu_iram.h | 3 +- cores/esp8266/umm_malloc/umm_malloc.h | 4 +- cores/esp8266/umm_malloc/umm_malloc_cfg.h | 82 ------------------- cores/esp8266/umm_malloc/umm_malloc_cfgport.h | 15 ++-- 8 files changed, 92 insertions(+), 96 deletions(-) create mode 100644 cores/esp8266/heap_api_debug.h diff --git a/cores/esp8266/Arduino.h b/cores/esp8266/Arduino.h index 52c6be4cd9..60737e0195 100644 --- a/cores/esp8266/Arduino.h +++ b/cores/esp8266/Arduino.h @@ -33,6 +33,7 @@ extern "C" { #include #include +#include "umm_malloc/umm_malloc_cfgport.h" #include "stdlib_noniso.h" #include "binary.h" #include "esp8266_peri.h" @@ -311,7 +312,8 @@ void configTime(const char* tz, String server1, #endif #ifdef DEBUG_ESP_OOM -// reinclude *alloc redefinition because of undefining them -// this is mandatory for allowing OOM *alloc definitions in .ino files -#include "umm_malloc/umm_malloc_cfg.h" +// Position *alloc redefinition at the end of Arduino.h because would +// have undefined them. Mandatory for supporting OOM and other debug alloc +// definitions in .ino files +#include "heap_api_debug.h" #endif diff --git a/cores/esp8266/Esp-frag.cpp b/cores/esp8266/Esp-frag.cpp index 249ff5770c..9e4e9af6f1 100644 --- a/cores/esp8266/Esp-frag.cpp +++ b/cores/esp8266/Esp-frag.cpp @@ -19,7 +19,6 @@ */ #include "umm_malloc/umm_malloc.h" -#include "umm_malloc/umm_malloc_cfg.h" #include "coredecls.h" #include "Esp.h" diff --git a/cores/esp8266/Esp.h b/cores/esp8266/Esp.h index b852890687..9f97175c29 100644 --- a/cores/esp8266/Esp.h +++ b/cores/esp8266/Esp.h @@ -22,7 +22,6 @@ #define ESP_H #include -#include "umm_malloc/umm_malloc_cfg.h" #include "core_esp8266_features.h" #include "spi_vendors.h" diff --git a/cores/esp8266/heap_api_debug.h b/cores/esp8266/heap_api_debug.h new file mode 100644 index 0000000000..62f7e7bad5 --- /dev/null +++ b/cores/esp8266/heap_api_debug.h @@ -0,0 +1,74 @@ +/* + * Issolated heap debug helper code from from umm_malloc/umm_malloc_cfg.h. + * Updated umm_malloc/umm_malloc.h and Arduino.h to reference. + * No #ifdef fenceing was used before. From its previous location, this content + * was reassert multiple times through Arduino.h. In case there are legacy + * projects that depend on the previous unfenced behavior, no fencing has been + * added. + */ + +/* + * *alloc redefinition - Included from Arduino.h for DEBUG_ESP_OOM support. + * + * It can also be directly include by the sketch for UMM_POISON_CHECK or + * UMM_POISON_CHECK_LITE builds to get more info about the caller when they + * report on a fail. + */ + +#ifdef __cplusplus +extern "C" { +#endif + +#ifdef DEBUG_ESP_OOM +#define MEMLEAK_DEBUG + +#include "umm_malloc/umm_malloc_cfg.h" + +#include +// Reuse pvPort* calls, since they already support passing location information. +// Specifically the debug version (heap_...) that does not force DRAM heap. +void *IRAM_ATTR heap_pvPortMalloc(size_t size, const char *file, int line); +void *IRAM_ATTR heap_pvPortCalloc(size_t count, size_t size, const char *file, int line); +void *IRAM_ATTR heap_pvPortRealloc(void *ptr, size_t size, const char *file, int line); +void *IRAM_ATTR heap_pvPortZalloc(size_t size, const char *file, int line); +void IRAM_ATTR heap_vPortFree(void *ptr, const char *file, int line); + +#define malloc(s) ({ static const char mem_debug_file[] PROGMEM STORE_ATTR = __FILE__; heap_pvPortMalloc(s, mem_debug_file, __LINE__); }) +#define calloc(n,s) ({ static const char mem_debug_file[] PROGMEM STORE_ATTR = __FILE__; heap_pvPortCalloc(n, s, mem_debug_file, __LINE__); }) +#define realloc(p,s) ({ static const char mem_debug_file[] PROGMEM STORE_ATTR = __FILE__; heap_pvPortRealloc(p, s, mem_debug_file, __LINE__); }) + +#if defined(UMM_POISON_CHECK) || defined(UMM_POISON_CHECK_LITE) +#define dbg_heap_free(p) ({ static const char mem_debug_file[] PROGMEM STORE_ATTR = __FILE__; heap_vPortFree(p, mem_debug_file, __LINE__); }) +#else +#define dbg_heap_free(p) free(p) +#endif + +#elif defined(UMM_POISON_CHECK) || defined(UMM_POISON_CHECK_LITE) // #elif for #ifdef DEBUG_ESP_OOM +#include +void *IRAM_ATTR heap_pvPortRealloc(void *ptr, size_t size, const char *file, int line); +#define realloc(p,s) ({ static const char mem_debug_file[] PROGMEM STORE_ATTR = __FILE__; heap_pvPortRealloc(p, s, mem_debug_file, __LINE__); }) + +void IRAM_ATTR heap_vPortFree(void *ptr, const char *file, int line); +// C - to be discussed +/* + Problem, I would like to report the file and line number with the umm poison + event as close as possible to the event. The #define method works for malloc, + calloc, and realloc those names are not as generic as free. A #define free + captures too much. Classes with methods called free are included :( + Inline functions would report the address of the inline function in the .h + not where they are called. + + Anybody know a trick to make this work? + + Create dbg_heap_free() as an alternative for free() when you need a little + more help in debugging the more challenging problems. +*/ +#define dbg_heap_free(p) ({ static const char mem_debug_file[] PROGMEM STORE_ATTR = __FILE__; heap_vPortFree(p, mem_debug_file, __LINE__); }) + +#else +#define dbg_heap_free(p) free(p) +#endif /* DEBUG_ESP_OOM */ + +#ifdef __cplusplus +} +#endif diff --git a/cores/esp8266/mmu_iram.h b/cores/esp8266/mmu_iram.h index 121226bd4c..bb4a33898b 100644 --- a/cores/esp8266/mmu_iram.h +++ b/cores/esp8266/mmu_iram.h @@ -56,7 +56,8 @@ extern "C" { #define DEV_DEBUG_PRINT */ -#if defined(DEV_DEBUG_PRINT) || defined(DEBUG_ESP_MMU) +#if (defined(DEV_DEBUG_PRINT) || defined(DEBUG_ESP_MMU)) && !defined(HOST_MOCK) +// Errors follow when `#include ` is present when running CI HOST #include #define DBG_MMU_FLUSH(a) while((USS(a) >> USTXC) & 0xff) {} diff --git a/cores/esp8266/umm_malloc/umm_malloc.h b/cores/esp8266/umm_malloc/umm_malloc.h index d3e3ace561..2c3b22cf74 100644 --- a/cores/esp8266/umm_malloc/umm_malloc.h +++ b/cores/esp8266/umm_malloc/umm_malloc.h @@ -10,8 +10,10 @@ #include -// C This include is not in upstream +// C These includes are not in the upstream #include "umm_malloc_cfg.h" /* user-dependent */ +#include +#include #ifdef __cplusplus extern "C" { diff --git a/cores/esp8266/umm_malloc/umm_malloc_cfg.h b/cores/esp8266/umm_malloc/umm_malloc_cfg.h index 706f0b7363..449d395fc2 100644 --- a/cores/esp8266/umm_malloc/umm_malloc_cfg.h +++ b/cores/esp8266/umm_malloc/umm_malloc_cfg.h @@ -695,91 +695,9 @@ struct UMM_TIME_STATS_t { UMM_TIME_STAT id_no_tag; }; #endif -///////////////////////////////////////////////// -#ifdef DEBUG_ESP_OOM - -#define MEMLEAK_DEBUG - -// umm_*alloc are not renamed to *alloc -// Assumes umm_malloc.h has already been included. - -#define umm_zalloc(s) umm_calloc(1,s) - -void *malloc_loc(size_t s, const char *file, int line); -void *calloc_loc(size_t n, size_t s, const char *file, int line); -void *realloc_loc(void *p, size_t s, const char *file, int line); -// *alloc are macro calling *alloc_loc calling+checking umm_*alloc() -// they are defined at the bottom of this file - -///////////////////////////////////////////////// - -#elif defined(UMM_POISON_CHECK) -void *realloc_loc(void *p, size_t s, const char *file, int line); -void free_loc(void *p, const char *file, int line); -#else // !defined(ESP_DEBUG_OOM) -#endif - - - #ifdef __cplusplus } #endif #endif /* _UMM_MALLOC_CFG_H */ - -#ifdef __cplusplus -extern "C" { -#endif -#ifdef DEBUG_ESP_OOM -// this must be outside from "#ifndef _UMM_MALLOC_CFG_H" -// because Arduino.h's does #undef *alloc -// Arduino.h recall us to redefine them -#include -// Reuse pvPort* calls, since they already support passing location information. -// Specifically the debug version (heap_...) that does not force DRAM heap. -void *IRAM_ATTR heap_pvPortMalloc(size_t size, const char *file, int line); -void *IRAM_ATTR heap_pvPortCalloc(size_t count, size_t size, const char *file, int line); -void *IRAM_ATTR heap_pvPortRealloc(void *ptr, size_t size, const char *file, int line); -void *IRAM_ATTR heap_pvPortZalloc(size_t size, const char *file, int line); -void IRAM_ATTR heap_vPortFree(void *ptr, const char *file, int line); - -#define malloc(s) ({ static const char mem_debug_file[] PROGMEM STORE_ATTR = __FILE__; heap_pvPortMalloc(s, mem_debug_file, __LINE__); }) -#define calloc(n,s) ({ static const char mem_debug_file[] PROGMEM STORE_ATTR = __FILE__; heap_pvPortCalloc(n, s, mem_debug_file, __LINE__); }) -#define realloc(p,s) ({ static const char mem_debug_file[] PROGMEM STORE_ATTR = __FILE__; heap_pvPortRealloc(p, s, mem_debug_file, __LINE__); }) - -#if defined(UMM_POISON_CHECK) || defined(UMM_POISON_CHECK_LITE) -#define dbg_heap_free(p) ({ static const char mem_debug_file[] PROGMEM STORE_ATTR = __FILE__; heap_vPortFree(p, mem_debug_file, __LINE__); }) -#else -#define dbg_heap_free(p) free(p) -#endif - -#elif defined(UMM_POISON_CHECK) || defined(UMM_POISON_CHECK_LITE) // #elif for #ifdef DEBUG_ESP_OOM -#include -void *IRAM_ATTR heap_pvPortRealloc(void *ptr, size_t size, const char *file, int line); -#define realloc(p,s) ({ static const char mem_debug_file[] PROGMEM STORE_ATTR = __FILE__; heap_pvPortRealloc(p, s, mem_debug_file, __LINE__); }) - -void IRAM_ATTR heap_vPortFree(void *ptr, const char *file, int line); -// C - to be discussed -/* - Problem, I would like to report the file and line number with the umm poison - event as close as possible to the event. The #define method works for malloc, - calloc, and realloc those names are not as generic as free. A #define free - captures too much. Classes with methods called free are included :( - Inline functions would report the address of the inline function in the .h - not where they are called. - - Anybody know a trick to make this work? - - Create dbg_heap_free() as an alternative for free() when you need a little - more help in debugging the more challenging problems. -*/ -#define dbg_heap_free(p) ({ static const char mem_debug_file[] PROGMEM STORE_ATTR = __FILE__; heap_vPortFree(p, mem_debug_file, __LINE__); }) - -#else -#define dbg_heap_free(p) free(p) -#endif /* DEBUG_ESP_OOM */ - -#ifdef __cplusplus -} -#endif diff --git a/cores/esp8266/umm_malloc/umm_malloc_cfgport.h b/cores/esp8266/umm_malloc/umm_malloc_cfgport.h index 8b463dea96..7dccbff932 100644 --- a/cores/esp8266/umm_malloc/umm_malloc_cfgport.h +++ b/cores/esp8266/umm_malloc/umm_malloc_cfgport.h @@ -1,13 +1,9 @@ -#ifndef _UMM_MALLOC_CFGPORT_H -#define _UMM_MALLOC_CFGPORT_H - -#ifndef _UMM_MALLOC_CFG_H -#error "This include file must be used with umm_malloc_cfg.h" -#endif - /* * Arduino ESP8266 core umm_malloc port config */ + +#ifdef _UMM_MALLOC_CFG_H +// Additional includes for "umm_malloc_cfg.h" only #include #include #include "../debug.h" @@ -18,6 +14,11 @@ #include #include "c_types.h" +#endif + + +#ifndef _UMM_MALLOC_CFGPORT_H +#define _UMM_MALLOC_CFGPORT_H /* * Between UMM_BEST_FIT or UMM_FIRST_FIT, UMM_BEST_FIT is the better option for