Skip to content

Commit

Permalink
Ensure xPortGetFreeHeapSize reports DRAM (#8680)
Browse files Browse the repository at this point in the history
Create dedicated function for xPortGetFreeHeapSize() that only reports on DRAM.
NONOS SDK API system_get_free_heap_size() relies on xPortGetFreeHeapSize() for the free Heap size.

Possible breaking change for multiple Heap Sketches calling system_get_free_heap_size(); it will now always report free DRAM Heap size.

Update and export umm_free_heap_size_lw() to report the free Heap size of the current Heap.
Updated ESP.getFreeHeap() to use umm_free_heap_size_lw().

Updated build options to supply exported umm_free_heap_size_lw() via either UMM_STATS or UMM_INFO.

Improved build option support via the SketchName.ino.globals.h method for Heap options: UMM_INFO, UMM_INLINE_METRICS, UMM_STATS, UMM_STATS_FULL, UMM_BEST_FIT, and UMM_FIRST_FIT. While uncommon to change from the defaults, you can review umm_malloc_cfgport.h for more details, which may help reduce your Sketch's size in dire situations. Assuming you are willing to give up some functionality.
For debugging UMM_STATS_FULL can offer additional stats, like Heap low water mark (umm_free_heap_size_min()).
  • Loading branch information
mhightower83 authored Oct 11, 2022
1 parent 7b2c627 commit d3eddeb
Show file tree
Hide file tree
Showing 14 changed files with 386 additions and 150 deletions.
8 changes: 5 additions & 3 deletions cores/esp8266/Arduino.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ extern "C" {
#include <string.h>
#include <math.h>

#include "umm_malloc/umm_malloc_cfgport.h"
#include "stdlib_noniso.h"
#include "binary.h"
#include "esp8266_peri.h"
Expand Down Expand Up @@ -311,7 +312,8 @@ void configTime(const char* tz, String server1,
#endif

#ifdef DEBUG_ESP_OOM
// reinclude *alloc redefinition because of <cstdlib> 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 <cstdlib> would
// have undefined them. Mandatory for supporting OOM and other debug alloc
// definitions in .ino files
#include "heap_api_debug.h"
#endif
3 changes: 2 additions & 1 deletion cores/esp8266/Esp-frag.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@
*/

#include "umm_malloc/umm_malloc.h"
#include "umm_malloc/umm_malloc_cfg.h"
#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.
Expand Down Expand Up @@ -60,3 +60,4 @@ uint8_t EspClass::getHeapFragmentation()
{
return (uint8_t)umm_fragmentation_metric();
}
#endif
4 changes: 3 additions & 1 deletion cores/esp8266/Esp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down
3 changes: 2 additions & 1 deletion cores/esp8266/Esp.h
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,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();

Expand Down
4 changes: 3 additions & 1 deletion cores/esp8266/heap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}

/*
Expand Down
74 changes: 74 additions & 0 deletions cores/esp8266/heap_api_debug.h
Original file line number Diff line number Diff line change
@@ -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 <pgmspace.h>
// 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 <pgmspace.h>
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
3 changes: 2 additions & 1 deletion cores/esp8266/mmu_iram.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 <esp8266_peri.h>` is present when running CI HOST
#include <esp8266_peri.h>

#define DBG_MMU_FLUSH(a) while((USS(a) >> USTXC) & 0xff) {}
Expand Down
66 changes: 66 additions & 0 deletions cores/esp8266/umm_malloc/Notes.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
8 changes: 8 additions & 0 deletions cores/esp8266/umm_malloc/umm_info.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
78 changes: 70 additions & 8 deletions cores/esp8266/umm_malloc/umm_local.c
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 3 additions & 1 deletion cores/esp8266/umm_malloc/umm_malloc.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@

#include <stdint.h>

// C This include is not in upstream
// C These includes are not in the upstream
#include "umm_malloc_cfg.h" /* user-dependent */
#include <osapi.h>
#include <heap_api_debug.h>

#ifdef __cplusplus
extern "C" {
Expand Down
Loading

0 comments on commit d3eddeb

Please sign in to comment.