8000 Reorganize local enhancements · esp8266/Arduino@47563cd · GitHub
[go: up one dir, main page]

Skip to content

Commit 47563cd

Browse files
committed
Reorganize local enhancements
More comment cleanups Added Status TODOs marks for upstream
1 parent 35c6868 commit 47563cd

File tree

5 files changed

+101
-84
lines changed

5 files changed

+101
-84
lines changed

cores/esp8266/umm_malloc/umm_info.c

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -157,18 +157,8 @@ void *umm_info( void *ptr, int force ) {
157157
}
158158
DBGLOG_FORCE( force, "+--------------------------------------------------------------+\n" );
159159

160-
DBGLOG_FORCE( force, "umm heap statistics:\n");
161-
DBGLOG_FORCE( force, " Free Space %5u\n", ummStats.free_blocks * sizeof(umm_block));
162-
#if defined(UMM_STATS_FULL)
163-
DBGLOG_FORCE( force, " Low Watermark %5u\n", ummStats.free_blocks_min * sizeof(umm_block));
164-
DBGLOG_FORCE( force, " Low Watermark ISR %5u\n", ummStats.free_blocks_isr_min * sizeof(umm_block));
165-
DBGLOG_FORCE( force, " MAX Alloc Request %5u\n", ummStats.alloc_max_size);
166-
DBGLOG_FORCE( force, " OOM Count %5u\n", ummStats.oom_count);
160+
print_stats(force);
167161
#endif
168-
DBGLOG_FORCE( force, " Size of umm_block %5u\n", sizeof(umm_block));
169-
DBGLOG_FORCE( force, "+--------------------------------------------------------------+\n" );
170-
#endif
171-
172162

173163
/* Release the critical section... */
174164
UMM_CRITICAL_EXIT(id_info);
@@ -178,7 +168,7 @@ void *umm_info( void *ptr, int force ) {
178168

179169
/* ------------------------------------------------------------------------ */
180170

181-
size_t umm_free_heap_size_info( void ) {
171+
size_t umm_free_heap_size( void ) {
182172
umm_info(NULL, 0);
183173
return (size_t)ummHeapInfo.freeBlocks * sizeof(umm_block);
184174
}
@@ -189,22 +179,7 @@ size_t umm_max_block_size( void ) {
189179
}
190180

191181
/* ------------------------------------------------------------------------ */
192-
#endif
193182

194-
#if defined(UMM_STATS) || defined(UMM_INFO)
195-
size_t umm_block_size( void ) {
196-
return sizeof(umm_block);
197-
}
198-
#endif
199-
200-
#if defined(UMM_STATS) || defined(UMM_STATS_FULL)
201-
202-
UMM_STATISTICS ummStats;
203-
204-
// Complete call path in IRAM
205-
size_t umm_free_heap_size_lw( void ) {
206-
return (size_t)ummStats.free_blocks * sizeof(umm_block);
207-
}
208183
#endif
209184

210185
#endif // defined(BUILD_UMM_MALLOC_C)

cores/esp8266/umm_malloc/umm_local.c

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,12 +149,51 @@ void umm_poison_free_fl(void *ptr, const char* file, int line) {
149149
}
150150
#endif
151151

152+
/* ------------------------------------------------------------------------ */
153+
154+
#if defined(UMM_STATS) || defined(UMM_STATS_FULL) || defined(UMM_INFO)
155+
size_t umm_block_size( void ) {
156+
return sizeof(umm_block);
157+
}
158+
#endif
159+
160+
#if defined(UMM_STATS) || defined(UMM_STATS_FULL)
161+
UMM_STATISTICS ummStats;
162+
163+
// Keep complete call path in IRAM
164+
size_t umm_free_heap_size_lw( void ) {
165+
return (size_t)ummStats.free_blocks * sizeof(umm_block);
166+
}
167+
#endif
168+
169+
/*
170+
I assume xPortGetFreeHeapSize needs to be in IRAM. Since
171+
system_get_free_heap_size is in IRAM. Which would mean, umm_free_heap_size()
172+
in flash, was not a safe alternative for returning the same information.
173+
*/
152174
#if defined(UMM_STATS) || defined(UMM_STATS_FULL)
153-
size_t umm_free_heap_size( void ) __attribute__ ((alias("umm_free_heap_size_lw")));
154175
size_t xPortGetFreeHeapSize(void) __attribute__ ((alias("umm_free_heap_size_lw")));
155176
#elif defined(UMM_INFO)
156-
size_t umm_free_heap_size( void ) __attribute__ ((alias("umm_free_heap_size_info")));
157-
size_t xPortGetFreeHeapSize(void) __attribute__ ((alias("umm_free_heap_size_info")));
177+
#warning "No ISR safe function available to implement xPortGetFreeHeapSize()"
178+
size_t xPortGetFreeHeapSize(void) __attribute__ ((alias("umm_free_heap_size")));
158179
#endif
159180

181+
#if defined(UMM_STATS) || defined(UMM_STATS_FULL)
182+
void print_stats(int force) {
183+
DBGLOG_FORCE( force, "umm heap statistics:\n");
184+
DBGLOG_FORCE( force, " Free Space %5u\n", ummStats.free_blocks * sizeof(umm_block));
185+
DBGLOG_FORCE( force, " OOM Count %5u\n", ummStats.oom_count);
186+
#if defined(UMM_STATS_FULL)
187+
DBGLOG_FORCE( force, " Low Watermark %5u\n", ummStats.free_blocks_min * sizeof(umm_block));
188+
DBGLOG_FORCE( force, " Low Watermark ISR %5u\n", ummStats.free_blocks_isr_min * sizeof(umm_block));
189+
DBGLOG_FORCE( force, " MAX Alloc Request %5u\n", ummStats.alloc_max_size);
190+
#endif
191+
DBGLOG_FORCE( force, " Size of umm_block %5u\n", sizeof(umm_block));
192+
DBGLOG_FORCE( force, "+--------------------------------------------------------------+\n" );
193+
}
160194
#endif
195+
196+
197+
#endif // BUILD_UMM_MALLOC_C
198+
199+

cores/esp8266/umm_malloc/umm_local.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,4 +34,8 @@
3434
static int check_poison_neighbors( unsigned short cur );
3535
#endif
3636

37+
#if defined(UMM_STATS) || defined(UMM_STATS_FULL)
38+
void ICACHE_FLASH_ATTR print_stats(int force);
39+
#endif
40+
3741
#endif

cores/esp8266/umm_malloc/umm_malloc.cpp

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,10 @@
4141
* In umm_malloc.c
4242
* Renamed to umm_malloc.cpp
4343
*
44-
* Added `extern "C" { ...b};` around code.
44+
* Added `extern "C" { ... };` around code.
4545
*
46-
* Surround DBGLOG_LEVEL with #ifndef... Now defined value in umm_malloc_cfg.h
47-
*
48-
* umm_free() - moved critical section to start after safe calculations.
49-
*
50-
* umm_malloc() - moved critical section to start after umm_blocks()
51-
* computations are based on constants that don't change, calling
52-
* argument excepted.
46+
* Surround DBGLOG_LEVEL with #ifndef... Define value of DBGLOG_LEVEL
47+
* from umm_malloc_cfg.h
5348
*
5449
* umm_realloc() - Added UMM_CRITICAL_SUSPEND()/UMM_CRITICAL_RESUME() for
5550
* when lightweight locks are available. eg. sti/cli. Single threaded
@@ -119,7 +114,7 @@
119114
* add to list to report.
120115
*/
121116
/*
122-
* Current Deltas from the old umm_malloc
117+
* New upstream umm_malloc feature delta's from the old umm_malloc we were using:
123118
*
124119
* umm_posion check for a given *alloc - failure - no longer panics.
125120
*
@@ -132,8 +127,10 @@
132127
* Defragmenting effect of realloc is gone. It now minimizes copy. This
133128
* may have been an accident during code cleanup.
134129
*
130+
* In one form or another these features have been restored in the
131+
* reintegration of the upstream umm_malloc into the Arduino ESP8266 Core.
135132
*/
136-
/* This block will be used for the PR description:
133+
/* This block will be used for the non-WIP PR description:
137134
138135
This updates the heap management library, umm_malloc, to the current upstream
139136
version at https://github.com/rhempel/umm_malloc. Some reorganizing and new code

cores/esp8266/umm_malloc/umm_malloc_cfg.h

Lines changed: 46 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -108,10 +108,8 @@ extern char _heap_start[];
108108
extern UMM_HEAP_INFO ummHeapInfo;
109109

110110
void ICACHE_FLASH_ATTR *umm_info( void *ptr, int force );
111-
size_t ICACHE_FLASH_ATTR umm_free_heap_size_info( void );
111+
size_t ICACHE_FLASH_ATTR umm_free_heap_size( void );
112112
size_t ICACHE_FLASH_ATTR umm_max_block_size( void );
113-
size_t ICACHE_FLASH_ATTR umm_block_size( void );
114-
115113
#else
116114
#endif
117115

@@ -127,13 +125,16 @@ extern char _heap_start[];
127125
*
128126
* UMM_INFO_PRINT is enabled as part of selecting `Debug port: "Serial" or
129127
* "Serial1"`. To make available all the time use '-D UMM_INFO_PRINT`.
128+
*
129+
* Status: Local Enhancement - addresses platform specific issue.
130130
*/
131-
#if defined(DEBUG_ESP_PORT) && !defined(UMM_INFO_PRINT)
131+
#if defined(DEBUG_ESP_PORT) && !defined(UMM_INFO_PRINT) && defined( 10669 UMM_INFO)
132132
#define UMM_INFO_PRINT
133133
#endif
134134

135135
/*
136136
* -D UMM_STATS :
137+
* -D UMM_STATS_FULL
137138
*
138139
* This option provides a lightweight alternative to using `umm_info` just for
139140
* getting `umm_free_heap_size`. With this option, a "free blocks" value is
@@ -144,18 +145,31 @@ extern char _heap_start[];
144145
* example is when an app closely monitors free heap to detect memory leaks. In
145146
* this case a single-core CPUs interrupt processing would have suffered the
146147
* most.
148+
*
149+
* UMM_STATS_FULL provides additional heap statistics. It can be used to gain
150+
* additional insight into heap usage. This option would add an additional 132
151+
* bytes of IRAM.
152+
*
153+
* Status: TODO: Needs to be proposed for upstream.
147154
*/
148155
/*
149156
#define UMM_STATS
150157
#define UMM_STATS_FULL
151158
*/
152159

153-
#if defined(DEBUG_ESP_PORT) && !defined(UMM_STATS) && !defined(UMM_STATS_FULL)
154-
#define UMM_STATS_FULL
155-
#elif !defined(UMM_STATS) && !defined(UMM_STATS_FULL)
160+
/*
161+
* For the ESP8266 we want at lest UMM_STATS built, so we have an ISR safe
162+
* function to call for implementing xPortGetFreeHeapSize(), because umm_info()
163+
* is in flash.
164+
*/
165+
#if !defined(UMM_STATS) && !defined(UMM_STATS_FULL)
156166
#define UMM_STATS
157167
#endif
158168

169+
#if defined(UMM_STATS) && defined(UMM_STATS_FULL)
170+
#undef UMM_STATS
171+
#endif
172+
159173
#if defined(UMM_STATS) || defined(UMM_STATS_FULL)
160174

161175
typedef struct UMM_STATISTICS_t {
@@ -191,6 +205,10 @@ static inline size_t ICACHE_FLASH_ATTR umm_get_oom_count( void ) {
191205
#define STATS__OOM_UPDATE() (void)0
192206
#endif
193207

208+
#if defined(UMM_STATS) || defined(UMM_STATS_FULL) || defined(UMM_INFO)
209+
size_t ICACHE_FLASH_ATTR umm_block_size( void );
210+
#endif
211+
194212
#ifdef UMM_STATS_FULL
195213
#define STATS__FREE_BLOCKS_MIN() \
196214
do { \
@@ -285,10 +303,6 @@ static inline size_t ICACHE_FLASH_ATTR umm_get_free_null_count( void ) {
285303
#define STATS__FREE_REQUEST(tag) (void)0
286304
#endif
287305

288-
#if defined(UMM_STATS) || defined(UMM_STATS_FULL) || defined(UMM_INFO)
289-
size_t umm_free_heap_size( void );
290-
#endif
291-
292306
/*
293307
Per Devyte, the core currently doesn't support masking a specific interrupt
294308
level. That doesn't mean it can't be implemented, only that at this time
@@ -306,11 +320,12 @@ size_t umm_free_heap_size( void );
306320
*
307321
* Build option to collect timing usage data on critical section usage in
308322
* functions: info, malloc, realloc. Collects MIN, MAX, and number of time IRQs
309-
* were disabled at request time. Note, for realloc MAX disabled time will not
310-
* include the time from calling malloc and/or free when UMM_LIGHTWEIGHT_CPU is
311-
* defined. Examine code for specifics on what info is available and how to
312-
* access.
313-
*/
323+
* were disabled at request time. Note, for realloc MAX disabled time will
324+
* include the time spent in calling malloc and/or free. Examine code for
325+
* specifics on what info is available and how to access.
326+
*
327+
* Status: TODO: Needs to be proposed for upstream.
328+
*/
314329
/*
315330
#define UMM_CRITICAL_METRICS
316331
*/
@@ -402,11 +417,12 @@ static inline void _critical_exit(UMM_TIME_STAT *p, uint32_t *saved_ps) {
402417
* UMM_CRITICAL_SUSPEND and UMM_CRITICAL_RESUME to the values of
403418
* UMM_CRITICAL_EXIT and UMM_CRITICAL_ENTRY. These additional exit/entries
404419
* allow time to service interrupts during the reentrant sections of the code.
405-
* Also, using these macros will relieve the nesting requirement.
406420
*
407-
* These macros should probably not be used on multicore CPUs. Hardware locking
408-
* methods sometimes carry higher overhead and may not be suitable for frequent
409-
* calling.
421+
* Performance may be impacked if used with multicore CPUs. The higher frquency
422+
* of locking and unlocking may be an issue with locking methods that have a
423+
* high overhead.
424+
*
425+
* Status: TODO: Needs to be proposed for upstream.
410426
*/
411427
/*
412428
*/
@@ -428,6 +444,10 @@ static inline void _critical_exit(UMM_TIME_STAT *p, uint32_t *saved_ps) {
428444
* shrinks an allocation, avoiding copy when possible. UMM_REALLOC_DEFRAG gives
429445
* priority with growing the revised allocation toward an adjacent hole in the
430446
* direction of the beginning of the heap when possible.
447+
*
448+
* Status: TODO: These are new options introduced to optionally restore the
449+
* previous defrag propery of realloc. The issue has been raised in the upstream
450+
* repo. No response at this time. Based on response, may propose for upstream.
431451
*/
432452
/*
433453
#define UMM_REALLOC_MINIMIZE_COPY
@@ -503,6 +523,11 @@ static inline void _critical_exit(UMM_TIME_STAT *p, uint32_t *saved_ps) {
503523
* include its nearest allocated neighbors in the heap.
504524
* umm_malloc() will also checks the neighbors of the selected allocation
505525
* before use.
526+
*
527+
* Status: TODO?: UMM_POISON_CHECK_LITE is a new option. We could propose for
528+
* upstream; however, the upstream version has much of the framework for calling
529+
* poison check on each alloc call refactored out. Not sure how this will be
530+
* received.
506531
*/
507532

508533
/*
@@ -571,6 +596,7 @@ static inline void _critical_exit(UMM_TIME_STAT *p, uint32_t *saved_ps) {
571596
// PROGMEM. Only the 1st parameter, fmt, is supported in PROGMEM.
572597
#define DBGLOG_FUNCTION(fmt, ...) ISR_PRINTF(fmt, ##__VA_ARGS__)
573598
#define DBGLOG_FUNCTION_P(fmt, ...) ISR_PRINTF_P(fmt, ##__VA_ARGS__)
599+
574600
#else
575601
#define DBGLOG_FUNCTION(fmt, ...) do { (void)fmt; } while(false)
576602
#define DBGLOG_FUNCTION_P(fmt, ...) do { (void)fmt; } while(false)
@@ -646,38 +672,14 @@ void* ICACHE_RAM_ATTR pvPortMalloc(size_t size, const char* file, int line);
646672
void* ICACHE_RAM_ATTR pvPortCalloc(size_t count, size_t size, const char* file, int line);
647673
void* ICACHE_RAM_ATTR pvPortRealloc(void *ptr, size_t size, const char* file, int line);
648674
void* ICACHE_RAM_ATTR pvPortZalloc(size_t size, const char* file, int line);
649-
void ICACHE_RAM_ATTR vPortFree(void *ptr, const char* file, int line);
650675
#define malloc(s) ({ static const char mem_debug_file[] PROGMEM STORE_ATTR = __FILE__; pvPortMalloc(s, mem_debug_file, __LINE__); })
651676
#define calloc(n,s) ({ static const char mem_debug_file[] PROGMEM STORE_ATTR = __FILE__; pvPortCalloc(n, s, mem_debug_file, __LINE__); })
652677
#define realloc(p,s) ({ static const char mem_debug_file[] PROGMEM STORE_ATTR = __FILE__; pvPortRealloc(p, s, mem_debug_file, __LINE__); })
653-
#if defined(UMM_POISON_CHECK) || defined(UMM_POISON_CHECK_LITE)
654-
#if 0
655-
#define free(p) ({ static const char mem_debug_file[] PROGMEM STORE_ATTR = __FILE__; vPortFree(p, mem_debug_file, __LINE__); })
656-
#endif
657-
#endif
658678

659679
#elif defined(UMM_POISON_CHECK) || defined(UMM_POISON_CHECK_LITE)
660680
#include <pgmspace.h>
661-
662681
void* ICACHE_RAM_ATTR pvPortRealloc(void *ptr, size_t size, const char* file, int line);
663-
void ICACHE_RAM_ATTR vPortFree(void *ptr, const char* file, int line);
664-
665682
#define realloc(p,s) ({ static const char mem_debug_file[] PROGMEM STORE_ATTR = __FILE__; pvPortRealloc(p, s, mem_debug_file, __LINE__); })
666-
#if 0
667-
//C
668-
/*
669-
Problem, I would like to report the file and line number with the umm poison
670-
event as close as possible to the event. The #define method works for malloc,
671-
calloc, and realloc those names are not as generic as "free". A #define free
672-
captures too much. Classes with methods called free are included :(
673-
Inline functions would report the address of the inline function in the .h
674-
not where they are called.
675-
676-
Anybody know a trick to make this work?
677-
*/
678-
#define free(p) ({ static const char mem_debug_file[] PROGMEM STORE_ATTR = __FILE__; vPortFree(p, mem_debug_file, __LINE__); })
679-
#endif
680-
681683
#endif /* DEBUG_ESP_OOM */
682684

683685
#ifdef __cplusplus

0 commit comments

Comments
 (0)
0