Ensure xPortGetFreeHeapSize reports DRAM

cores/esp8266/Esp-frag.cpp

@@ -23,6 +23,7 @@
 #include "coredecls.h"
 #include "Esp.h"
 
+#if defined(UMM_INFO)
 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

cores/esp8266/Esp.cpp

@@ -219,13 +219,15 @@ uint16_t EspClass::getVcc(void)
 
 uint32_t EspClass::getFreeHeap(void)
 {
-    return system_get_free_heap_size();
+    return umm_free_heap_size_lw();
 }
 
+#if defined(UMM_INFO)
 uint32_t EspClass::getMaxFreeBlockSize(void)
 {
     return umm_max_block_size();
 }
+#endif
 
 uint32_t EspClass::getFreeContStack()
 {

cores/esp8266/Esp.h

@@ -22,6 +22,7 @@
 #define ESP_H
 
 #include <Arduino.h>
+#include "umm_malloc/umm_malloc_cfg.h"
 #include "core_esp8266_features.h"
 #include "spi_vendors.h"
 
@@ -114,11 +115,12 @@ 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);
-
+#endif
         static uint32_t getFreeContStack();
         static void resetFreeContStack();
 

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
 }
 
 /*

cores/esp8266/umm_malloc/Notes.h

@@ -276,4 +276,70 @@ 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_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
+  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:
+
+  * 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 `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_lw()`
+      * For multiple Heaps builds, add a dedicated function that always reports
+        DRAM results.
+
+*/
 #endif

cores/esp8266/umm_malloc/umm_info.c

@@ -174,6 +174,14 @@ size_t umm_free_heap_size_core(umm_heap_context_t *_context) {
     return (size_t)_context->info.freeBlocks * sizeof(umm_block);
 }
 
+/*
+  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(void) {
     #ifndef UMM_INLINE_METRICS
     umm_info(NULL, false);

cores/esp8266/umm_malloc/umm_local.c

@@ -161,35 +161,97 @@ 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)
-// Keep complete call path in IRAM
+/*
+  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();
     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.
+  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 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 xPortGetFreeHeapSize(void) __attribute__ ((alias("umm_free_heap_size_lw")));
+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)
-#ifndef UMM_INLINE_METRICS
-#warning "No ISR safe function available to implement xPortGetFreeHeapSize()"
+// 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. 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 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);
+
+    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.
+    // Not in IRAM, umm_info() would have been used to complete this operation.
+    #error "No ISR safe function available to implement xPortGetFreeHeapSize()"
+    #endif
+}
+#endif
+
 #if defined(UMM_STATS) || defined(UMM_STATS_FULL)
 void umm_print_stats(int force) {
     umm_heap_context_t *_context = umm_get_current_heap();

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,32 +194,23 @@ 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);
-#ifdef UMM_INLINE_METRICS
-extern size_t umm_free_heap_size(void);
-#else
+#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);
-#else
-  #define umm_info(p,b)
-  #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() (0)
-  #define umm_max_block_size_core() (0)
-  #define umm_fragmentation_metric_core() (0)
 #endif
 
 /*
@@ -305,7 +281,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