umm_malloc manual merge with upstream (#7337)

* umm_malloc manual merge with upstream

* Fix divide by zero, case when heap is 100% allocated.

* Removed extra line.

* Fixed block count for debug build. This resolves OOM events for debug build.
Correct overstepping array when freeing.

* Handle another corner case in example HeapMetric.ino.
Comment corrections.

* Revert - ESP.getMaxFreeBlockSize() is back to indicating the size of a
contiguous block of memory before the umm_malloc overhead is removed.

* Stale code cleanup and comment improvements

Author: mhightower83

cores/esp8266/Esp-frag.cpp

@@ -29,20 +29,28 @@ void EspClass::getHeapStats(uint32_t* hfree, uint16_t* hmax, uint8_t* hfrag)
     // Having getFreeHeap()=sum(hole-size), fragmentation is given by
     // 100 * (1 - sqrt(sum(hole-size²)) / sum(hole-size))
 
-    umm_info(NULL, 0);
+    umm_info(NULL, false);
     uint8_t block_size = umm_block_size();
-    uint32_t fh = ummHeapInfo.freeBlocks * block_size;
     if (hfree)
-        *hfree = fh;
+        *hfree = ummHeapInfo.freeBlocks * block_size;
     if (hmax)
-        *hmax = ummHeapInfo.maxFreeContiguousBlocks * block_size;
-    if (hfrag)
-        *hfrag = 100 - (sqrt32(ummHeapInfo.freeSize2) * 100) / fh;
+        *hmax = (uint16_t)ummHeapInfo.maxFreeContiguousBlocks * block_size;
+    if (hfrag) {
+      if (ummHeapInfo.freeBlocks) {
+        *hfrag = 100 - (sqrt32(ummHeapInfo.freeBlocksSquared) * 100) / ummHeapInfo.freeBlocks;
+      } else {
+        *hfrag = 0;
+      }
+    }
 }
 
 uint8_t EspClass::getHeapFragmentation()
 {
+#ifdef UMM_INLINE_METRICS
+    return (uint8_t)umm_fragmentation_metric();
+#else
     uint8_t hfrag;
     getHeapStats(nullptr, nullptr, &hfrag);
     return hfrag;
+#endif
 }

cores/esp8266/heap.cpp

@@ -314,7 +314,7 @@ size_t ICACHE_RAM_ATTR xPortWantedSizeAlign(size_t size)
 
 void system_show_malloc(void)
 {
-    umm_info(NULL, 1);
+    umm_info(NULL, true);
 }
 
 };

cores/esp8266/umm_malloc/dbglog/dbglog.h

@@ -11,7 +11,7 @@
  * ----------------------------------------------------------------------------
  * NOTE WELL that this file may be included multiple times - this allows you
  * to set the trace level #define DBGLOG_LEVEL x
- * 
+ *
  * To update which of the DBGLOG macros are compiled in, you must redefine the
  * DBGLOG_LEVEL macro and the inlcude the dbglog.h file again, like this:
  *
@@ -57,6 +57,8 @@
 #  define DBGLOG_FUNCTION printf
 #endif
 
+#define DBGLOG_32_BIT_PTR(x) ((uint32_t)(((uintptr_t)(x)) & 0xffffffff))
+
 /* ------------------------------------------------------------------------- */
 
 #if DBGLOG_LEVEL >= 6

cores/esp8266/umm_malloc/umm_info.c

@@ -2,6 +2,12 @@
 
 #ifdef UMM_INFO
 
+#include <stdint.h>
+#include <stddef.h>
+#include <stdbool.h>
+
+#include <math.h>
+
 /* ----------------------------------------------------------------------------
  * One of the coolest things about this little library is that it's VERY
  * easy to get debug information about the memory heap by simply iterating
@@ -19,15 +25,15 @@
 
 UMM_HEAP_INFO ummHeapInfo;
 
-void *umm_info( void *ptr, int force ) {
+void *umm_info( void *ptr, bool force ) {
   UMM_CRITICAL_DECL(id_info);
 
-  unsigned short int blockNo = 0;
-
-  if (umm_heap == NULL) {
+  if(umm_heap == NULL) {
     umm_init();
   }
 
+  uint16_t blockNo = 0;
+
   /* Protect the critical section... */
   UMM_CRITICAL_ENTRY(id_info);
 
@@ -40,7 +46,7 @@ void *umm_info( void *ptr, int force ) {
   DBGLOG_FORCE( force, "\n" );
   DBGLOG_FORCE( force, "+----------+-------+--------+--------+-------+--------+--------+\n" );
   DBGLOG_FORCE( force, "|0x%08lx|B %5d|NB %5d|PB %5d|Z %5d|NF %5d|PF %5d|\n",
-      (unsigned long)(&UMM_BLOCK(blockNo)),
+      DBGLOG_32_BIT_PTR(&UMM_BLOCK(blockNo)),
       blockNo,
       UMM_NBLOCK(blockNo) & UMM_BLOCKNO_MASK,
       UMM_PBLOCK(blockNo),
@@ -67,21 +73,18 @@ void *umm_info( void *ptr, int force ) {
     if( UMM_NBLOCK(blockNo) & UMM_FREELIST_MASK ) {
       ++ummHeapInfo.freeEntries;
       ummHeapInfo.freeBlocks += curBlocks;
-      ummHeapInfo.freeSize2 += (unsigned int)curBlocks
-                              * (unsigned int)sizeof(umm_block)
-                              * (unsigned int)curBlocks
-                              * (unsigned int)sizeof(umm_block);
+      ummHeapInfo.freeBlocksSquared += (curBlocks * curBlocks);
 
       if (ummHeapInfo.maxFreeContiguousBlocks < curBlocks) {
         ummHeapInfo.maxFreeContiguousBlocks = curBlocks;
       }
 
       DBGLOG_FORCE( force, "|0x%08lx|B %5d|NB %5d|PB %5d|Z %5u|NF %5d|PF %5d|\n",
-          (unsigned long)(&UMM_BLOCK(blockNo)),
+          DBGLOG_32_BIT_PTR(&UMM_BLOCK(blockNo)),
           blockNo,
           UMM_NBLOCK(blockNo) & UMM_BLOCKNO_MASK,
           UMM_PBLOCK(blockNo),
-          (unsigned int)curBlocks,
+          (uint16_t)curBlocks,
           UMM_NFREE(blockNo),
           UMM_PFREE(blockNo) );
 
@@ -99,33 +102,25 @@ void *umm_info( void *ptr, int force ) {
       ummHeapInfo.usedBlocks += curBlocks;
 
       DBGLOG_FORCE( force, "|0x%08lx|B %5d|NB %5d|PB %5d|Z %5u|\n",
-          (unsigned long)(&UMM_BLOCK(blockNo)),
+          DBGLOG_32_BIT_PTR(&UMM_BLOCK(blockNo)),
           blockNo,
           UMM_NBLOCK(blockNo) & UMM_BLOCKNO_MASK,
           UMM_PBLOCK(blockNo),
-          (unsigned int)curBlocks );
+          (uint16_t)curBlocks );
     }
 
     blockNo = UMM_NBLOCK(blockNo) & UMM_BLOCKNO_MASK;
   }
 
   /*
-   * Update the accounting totals with information from the last block, the
-   * rest must be free!
+   * The very last block is used as a placeholder to indicate that
+   * there are no more blocks in the heap, so it cannot be used
+   * for anything - at the same time, the size of this block must
+   * ALWAYS be exactly 1 !
    */
 
-  {
-    size_t curBlocks = UMM_NUMBLOCKS-blockNo;
-    ummHeapInfo.freeBlocks  += curBlocks;
-    ummHeapInfo.totalBlocks += curBlocks;
-
-    if (ummHeapInfo.maxFreeContiguousBlocks < curBlocks) {
-      ummHeapInfo.maxFreeContiguousBlocks = curBlocks;
-    }
-  }
-
   DBGLOG_FORCE( force, "|0x%08lx|B %5d|NB %5d|PB %5d|Z %5d|NF %5d|PF %5d|\n",
-      (unsigned long)(&UMM_BLOCK(blockNo)),
+      DBGLOG_32_BIT_PTR(&UMM_BLOCK(blockNo)),
       blockNo,
       UMM_NBLOCK(blockNo) & UMM_BLOCKNO_MASK,
       UMM_PBLOCK(blockNo),
@@ -147,7 +142,13 @@ void *umm_info( void *ptr, int force ) {
 
   DBGLOG_FORCE( force, "+--------------------------------------------------------------+\n" );
 
+  DBGLOG_FORCE( force, "Usage Metric:               %5d\n", umm_usage_metric());
+  DBGLOG_FORCE( force, "Fragmentation Metric:       %5d\n", umm_fragmentation_metric());
+
+  DBGLOG_FORCE( force, "+--------------------------------------------------------------+\n" );
+
 #if defined(UMM_STATS) || defined(UMM_STATS_FULL)
+#if !defined(UMM_INLINE_METRICS)
   if (ummHeapInfo.freeBlocks == ummStats.free_blocks) {
       DBGLOG_FORCE( force, "heap info Free blocks and heap statistics Free blocks match.\n");
   } else {
@@ -156,6 +157,7 @@ void *umm_info( void *ptr, int force ) {
           ummStats.free_blocks  );
   }
   DBGLOG_FORCE( force, "+--------------------------------------------------------------+\n" );
+#endif
 
   print_stats(force);
 #endif
@@ -169,17 +171,74 @@ void *umm_info( void *ptr, int force ) {
 /* ------------------------------------------------------------------------ */
 
 size_t umm_free_heap_size( void ) {
-  umm_info(NULL, 0);
+#ifndef UMM_INLINE_METRICS
+  umm_info(NULL, false);
+#endif
   return (size_t)ummHeapInfo.freeBlocks * sizeof(umm_block);
 }
 
+//C Breaking change in upstream umm_max_block_size() was changed to
+//C umm_max_free_block_size() keeping old function name for (dot) releases.
+//C TODO: update at next major release.
+//C size_t umm_max_free_block_size( void ) {
 size_t umm_max_block_size( void ) {
-  umm_info(NULL, 0);
+  umm_info(NULL, false);
   return ummHeapInfo.maxFreeContiguousBlocks * sizeof(umm_block);
 }
 
-/* ------------------------------------------------------------------------ */
+/*
+  Without build option UMM_INLINE_METRICS, calls to umm_usage_metric() or
+  umm_fragmentation_metric() must to be preceeded by a call to umm_info(NULL, false)
+  for updated results.
+*/
+int umm_usage_metric( void ) {
+#ifndef UMM_INLINE_METRICS
+  umm_info(NULL, false);
+#endif
+  DBGLOG_DEBUG( "usedBlocks %d totalBlocks %d\n", umm_metrics.usedBlocks, ummHeapInfo.totalBlocks);
+  if (ummHeapInfo.freeBlocks)
+    return (int)((ummHeapInfo.usedBlocks * 100)/(ummHeapInfo.freeBlocks));
 
+  return -1;  // no freeBlocks
+}
+
+uint32_t sqrt32 (uint32_t n);
+
+int umm_fragmentation_metric( void ) {
+#ifndef UMM_INLINE_METRICS
+  umm_info(NULL, false);
+#endif
+  DBGLOG_DEBUG( "freeBlocks %d freeBlocksSquared %d\n", umm_metrics.freeBlocks, ummHeapInfo.freeBlocksSquared);
+  if (0 == ummHeapInfo.freeBlocks) {
+      return 0;
+  } else {
+      //upstream version: return (100 - (((uint32_t)(sqrtf(ummHeapInfo.freeBlocksSquared)) * 100)/(ummHeapInfo.freeBlocks)));
+      return (100 - (((uint32_t)(sqrt32(ummHeapInfo.freeBlocksSquared)) * 100)/(ummHeapInfo.freeBlocks)));
+  }
+}
+
+#ifdef UMM_INLINE_METRICS
+static void umm_fragmentation_metric_init( void ) {
+    ummHeapInfo.freeBlocks = UMM_NUMBLOCKS - 2;
+    ummHeapInfo.freeBlocksSquared = ummHeapInfo.freeBlocks * ummHeapInfo.freeBlocks;
+}
+
+static void umm_fragmentation_metric_add( uint16_t c ) {
+    uint16_t blocks = (UMM_NBLOCK(c) & UMM_BLOCKNO_MASK) - c;
+    DBGLOG_DEBUG( "Add block %d size %d to free metric\n", c, blocks);
+    ummHeapInfo.freeBlocks += blocks;
+    ummHeapInfo.freeBlocksSquared += (blocks * blocks);
+}
+
+static void umm_fragmentation_metric_remove( uint16_t c ) {
+    uint16_t blocks = (UMM_NBLOCK(c) & UMM_BLOCKNO_MASK) - c;
+    DBGLOG_DEBUG( "Remove block %d size %d from free metric\n", c, blocks);
+    ummHeapInfo.freeBlocks -= blocks;
+    ummHeapInfo.freeBlocksSquared -= (blocks * blocks);
+}
+#endif // UMM_INLINE_METRICS
+
+/* ------------------------------------------------------------------------ */
 #endif
 
 #endif  // defined(BUILD_UMM_MALLOC_C)

cores/esp8266/umm_malloc/umm_integrity.c

@@ -1,6 +1,10 @@
 #if defined(BUILD_UMM_MALLOC_C)
 /* integrity check (UMM_INTEGRITY_CHECK) {{{ */
 #if defined(UMM_INTEGRITY_CHECK)
+
+#include <stdint.h>
+#include <stdbool.h>
+
 /*
  * Perform integrity check of the whole heap data. Returns 1 in case of
  * success, 0 otherwise.
@@ -23,11 +27,11 @@
  * This way, we ensure that the free flag is in sync with the free pointers
  * chain.
  */
-int umm_integrity_check(void) {
+bool umm_integrity_check(void) {
   UMM_CRITICAL_DECL(id_integrity);
-  int ok = 1;
-  unsigned short int prev;
-  unsigned short int cur;
+  bool ok = true;
+  uint16_t prev;
+  uint16_t cur;
 
   if (umm_heap == NULL) {
     umm_init();
@@ -42,9 +46,9 @@ int umm_integrity_check(void) {
     /* Check that next free block number is valid */
     if (cur >= UMM_NUMBLOCKS) {
       DBGLOG_FUNCTION("heap integrity broken: too large next free num: %d "
-          "(in block %d, addr 0x%lx)\n", cur, prev,
-          (unsigned long)&UMM_NBLOCK(prev));
-      ok = 0;
+          "(in block %d, addr 0x%08x)\n", cur, prev,
+          DBGLOG_32_BIT_PTR(&UMM_NBLOCK(prev)));
+      ok = false;
       goto clean;
     }
     if (cur == 0) {
@@ -57,7 +61,7 @@ int umm_integrity_check(void) {
       DBGLOG_FUNCTION("heap integrity broken: free links don't match: "
           "%d -> %d, but %d -> %d\n",
           prev, cur, cur, UMM_PFREE(cur));
-      ok = 0;
+      ok = false;
       goto clean;
     }
 
@@ -74,9 +78,9 @@ int umm_integrity_check(void) {
     /* Check that next block number is valid */
     if (cur >= UMM_NUMBLOCKS) {
       DBGLOG_FUNCTION("heap integrity broken: too large next block num: %d "
-          "(in block %d, addr 0x%lx)\n", cur, prev,
-          (unsigned long)&UMM_NBLOCK(prev));
-      ok = 0;
+          "(in block %d, addr 0x%08x)\n", cur, prev,
+          DBGLOG_32_BIT_PTR(&UMM_NBLOCK(prev)));
+      ok = false;
       goto clean;
     }
     if (cur == 0) {
@@ -88,21 +92,20 @@ int umm_integrity_check(void) {
     if ((UMM_NBLOCK(cur) & UMM_FREELIST_MASK)
         != (UMM_PBLOCK(cur) & UMM_FREELIST_MASK))
     {
-      DBGLOG_FUNCTION("heap integrity broken: mask wrong at addr 0x%lx: n=0x%x, p=0x%x\n",
-          (unsigned long)&UMM_NBLOCK(cur),
+      DBGLOG_FUNCTION("heap integrity broken: mask wrong at addr 0x%08x: n=0x%x, p=0x%x\n",
+          DBGLOG_32_BIT_PTR(&UMM_NBLOCK(cur)),
           (UMM_NBLOCK(cur) & UMM_FREELIST_MASK),
-          (UMM_PBLOCK(cur) & UMM_FREELIST_MASK)
-          );
-      ok = 0;
+          (UMM_PBLOCK(cur) & UMM_FREELIST_MASK));
+      ok = false;
       goto clean;
     }
 
     /* make sure the block list is sequential */
     if (cur <= prev ) {
      DBGLOG_FUNCTION("heap integrity broken: next block %d is before prev this one "
-          "(in block %d, addr 0x%lx)\n", cur, prev,
-          (unsigned long)&UMM_NBLOCK(prev));
-      ok = 0;
+          "(in block %d, addr 0x%08x)\n", cur, prev,
+          DBGLOG_32_BIT_PTR(&UMM_NBLOCK(prev)));
+      ok = false;
       goto clean;
     }
 
@@ -114,7 +117,7 @@ int umm_integrity_check(void) {
       DBGLOG_FUNCTION("heap integrity broken: block links don't match: "
           "%d -> %d, but %d -> %d\n",
           prev, cur, cur, UMM_PBLOCK(cur));
-      ok = 0;
+      ok = false;
       goto clean;
     }