From 7a2beda2a976777a12829d31917e5404832d05bf Mon Sep 17 00:00:00 2001 From: Neil McKechnie Date: Mon, 8 Mar 2021 15:32:40 +0000 Subject: [PATCH 1/9] Add optional loop time monitor. By defining ENABLE_LOOP_MEASUREMENT as true in config.h, the loop measurement will be enabled. This measures the time between successive executions of the main CS loop to help identify if something is taking too long and holding up the other loop functions. --- CommandStation-EX.ino | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/CommandStation-EX.ino b/CommandStation-EX.ino index 2357638..1fe625a 100644 --- a/CommandStation-EX.ino +++ b/CommandStation-EX.ino @@ -134,4 +134,31 @@ void loop() LCD(2,F("Free RAM=%5db"), ramLowWatermark); } #endif + +// Optionally report average and maximum loop time +#if ENABLE_LOOP_MEASUREMENT + static unsigned long startTime = micros(); + static unsigned int maxElapsed = 0; + static unsigned long totalElapsed = 0; + static unsigned long count = 0; + static unsigned long lastOutput = millis(); + + unsigned long endTime = micros(); + unsigned int elapsed = endTime - startTime; + + if (elapsed > maxElapsed) maxElapsed = elapsed; + count++; + totalElapsed += elapsed; + + unsigned long currentMillis = millis(); + if (currentMillis - lastOutput >= 5000) { + DIAG(F("\nLoop: max=%dus ave=%dus\n"), maxElapsed, totalElapsed/count); + maxElapsed = 0; + totalElapsed = 0; + count = 0; + lastOutput = currentMillis; + } + startTime = micros(); +#endif + } From fab05bac798015cf7a651fb8cea048b154aa8230 Mon Sep 17 00:00:00 2001 From: Neil McKechnie Date: Tue, 9 Mar 2021 10:13:04 +0000 Subject: [PATCH 2/9] Update freeMemory.cpp Inhibit interrupts while updating/reading minimum_free_memory as it is accessed from interrupt handler. --- freeMemory.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/freeMemory.cpp b/freeMemory.cpp index 6735ac1..500cff8 100644 --- a/freeMemory.cpp +++ b/freeMemory.cpp @@ -18,6 +18,7 @@ * along with CommandStation. If not, see . */ +#include #include "freeMemory.h" // thanks go to https://github.com/mpflaga/Arduino-MemoryFree @@ -50,6 +51,10 @@ int freeMemory() { // called subroutines. int updateMinimumFreeMemory(unsigned char extraBytes) { int spare = freeMemory()-extraBytes; + byte sreg_save = SREG; + noInterrupts(); if (spare < minimum_free_memory) minimum_free_memory = spare; - return minimum_free_memory; + int returnValue = minimum_free_memory; + SREG = sreg_save; + return returnValue; } From 7954c85b7daae2de9ab894d0b34d0646e5356125 Mon Sep 17 00:00:00 2001 From: Neil McKechnie Date: Tue, 9 Mar 2021 10:27:38 +0000 Subject: [PATCH 3/9] Update freeMemory.cpp --- freeMemory.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/freeMemory.cpp b/freeMemory.cpp index 500cff8..fd99d5b 100644 --- a/freeMemory.cpp +++ b/freeMemory.cpp @@ -50,9 +50,9 @@ int freeMemory() { // by estimation or inspection, that may be used by other // called subroutines. int updateMinimumFreeMemory(unsigned char extraBytes) { - int spare = freeMemory()-extraBytes; byte sreg_save = SREG; noInterrupts(); + int spare = freeMemory()-extraBytes; if (spare < minimum_free_memory) minimum_free_memory = spare; int returnValue = minimum_free_memory; SREG = sreg_save; From 62f1c04ee3e8605e3851a47956854ff5d9674267 Mon Sep 17 00:00:00 2001 From: Neil McKechnie Date: Tue, 9 Mar 2021 10:30:20 +0000 Subject: [PATCH 4/9] Revert "Add optional loop time monitor." This reverts commit 7a2beda2a976777a12829d31917e5404832d05bf. --- CommandStation-EX.ino | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/CommandStation-EX.ino b/CommandStation-EX.ino index 1fe625a..2357638 100644 --- a/CommandStation-EX.ino +++ b/CommandStation-EX.ino @@ -134,31 +134,4 @@ void loop() LCD(2,F("Free RAM=%5db"), ramLowWatermark); } #endif - -// Optionally report average and maximum loop time -#if ENABLE_LOOP_MEASUREMENT - static unsigned long startTime = micros(); - static unsigned int maxElapsed = 0; - static unsigned long totalElapsed = 0; - static unsigned long count = 0; - static unsigned long lastOutput = millis(); - - unsigned long endTime = micros(); - unsigned int elapsed = endTime - startTime; - - if (elapsed > maxElapsed) maxElapsed = elapsed; - count++; - totalElapsed += elapsed; - - unsigned long currentMillis = millis(); - if (currentMillis - lastOutput >= 5000) { - DIAG(F("\nLoop: max=%dus ave=%dus\n"), maxElapsed, totalElapsed/count); - maxElapsed = 0; - totalElapsed = 0; - count = 0; - lastOutput = currentMillis; - } - startTime = micros(); -#endif - } From 0880507d89c18e0c3547b0ba981c7658ccfe9406 Mon Sep 17 00:00:00 2001 From: Neil McKechnie Date: Tue, 9 Mar 2021 10:38:48 +0000 Subject: [PATCH 5/9] Make memory monitoring non-optional. --- CommandStation-EX.ino | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/CommandStation-EX.ino b/CommandStation-EX.ino index 2357638..14b0057 100644 --- a/CommandStation-EX.ino +++ b/CommandStation-EX.ino @@ -123,8 +123,7 @@ void loop() LCDDisplay::loop(); // ignored if LCD not in use -// Optionally report any decrease in memory (will automatically trigger on first call) -#if ENABLE_FREE_MEM_WARNING +// Report any decrease in memory (will automatically trigger on first call) static int ramLowWatermark = 32767; // replaced on first loop int freeNow = updateMinimumFreeMemory(); @@ -133,5 +132,4 @@ void loop() ramLowWatermark = freeNow; LCD(2,F("Free RAM=%5db"), ramLowWatermark); } -#endif } From 163dd270e89214cd4203ee3157cc054f8cc0a377 Mon Sep 17 00:00:00 2001 From: Neil McKechnie Date: Tue, 9 Mar 2021 22:43:41 +0000 Subject: [PATCH 6/9] Memory monitoring updates Split update from read value; Inhibit interrupts when reading (normally done from loop code); Don't inhibit interrupts when updating (normally done from interupt code); Make freeMemory() local and ask for inline code generation. --- CommandStation-EX.ino | 2 +- DCCEXParser.cpp | 2 +- freeMemory.cpp | 23 ++++++++++++++--------- freeMemory.h | 4 ++-- 4 files changed, 18 insertions(+), 13 deletions(-) diff --git a/CommandStation-EX.ino b/CommandStation-EX.ino index 14b0057..4954223 100644 --- a/CommandStation-EX.ino +++ b/CommandStation-EX.ino @@ -126,7 +126,7 @@ void loop() // Report any decrease in memory (will automatically trigger on first call) static int ramLowWatermark = 32767; // replaced on first loop - int freeNow = updateMinimumFreeMemory(); + int freeNow = minimumFreeMemory(); if (freeNow < ramLowWatermark) { ramLowWatermark = freeNow; diff --git a/DCCEXParser.cpp b/DCCEXParser.cpp index e0496cc..2556cc2 100644 --- a/DCCEXParser.cpp +++ b/DCCEXParser.cpp @@ -720,7 +720,7 @@ bool DCCEXParser::parseD(Print *stream, int params, int p[]) return true; case HASH_KEYWORD_RAM: // - StringFormatter::send(stream, F("\nFree memory=%d\n"), freeMemory()); + StringFormatter::send(stream, F("\nFree memory=%d\n"), minimumFreeMemory()); break; case HASH_KEYWORD_ACK: // diff --git a/freeMemory.cpp b/freeMemory.cpp index fd99d5b..f8d2a0b 100644 --- a/freeMemory.cpp +++ b/freeMemory.cpp @@ -35,26 +35,31 @@ extern char *__malloc_heap_start; static volatile int minimum_free_memory = 32767; -int freeMemory() { +static inline int freeMemory() { char top; #if defined(__arm__) return &top - reinterpret_cast(sbrk(0)); #elif defined(__AVR__) return __brkval ? &top - __brkval : &top - __malloc_heap_start; #else -#error bailed out alredy above +#error bailed out already above #endif } // Update low ram level. Allow for extra bytes to be specified // by estimation or inspection, that may be used by other -// called subroutines. -int updateMinimumFreeMemory(unsigned char extraBytes) { - byte sreg_save = SREG; - noInterrupts(); +// called subroutines. Must be called with interrupts disabled. +// +void updateMinimumFreeMemory(unsigned char extraBytes) { int spare = freeMemory()-extraBytes; if (spare < minimum_free_memory) minimum_free_memory = spare; - int returnValue = minimum_free_memory; - SREG = sreg_save; - return returnValue; +} + +// Return low memory value. +int minimumFreeMemory() { + byte sreg_save = SREG; + noInterrupts(); // Disable interrupts + int retval = minimum_free_memory; + SREG = sreg_save; // Restore interrupt state + return retval; } diff --git a/freeMemory.h b/freeMemory.h index bf93189..3d1fe40 100644 --- a/freeMemory.h +++ b/freeMemory.h @@ -20,6 +20,6 @@ #ifndef freeMemory_h #define freeMemory_h -int freeMemory(); -int updateMinimumFreeMemory(unsigned char extraBytes=0); +void updateMinimumFreeMemory(unsigned char extraBytes=0); +int minimumFreeMemory(); #endif From def6c24baccc55695f5e4ed00cc0433c28c69aa4 Mon Sep 17 00:00:00 2001 From: Neil McKechnie Date: Tue, 9 Mar 2021 23:39:25 +0000 Subject: [PATCH 7/9] Remove redundant option for memory monitoring. Memory monitoring now enabled always. --- config.example.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/config.example.h b/config.example.h index f528b3e..b4751b5 100644 --- a/config.example.h +++ b/config.example.h @@ -111,6 +111,4 @@ The configuration file for DCC-EX Command Station // #define OLED_DRIVER 128,32 ///////////////////////////////////////////////////////////////////////////////////// -// -// Enable warning as memory gets depleted -#define ENABLE_FREE_MEM_WARNING false + From eb54c78d740824e677567fef24ac3917e7e17173 Mon Sep 17 00:00:00 2001 From: Neil McKechnie Date: Tue, 9 Mar 2021 23:41:33 +0000 Subject: [PATCH 8/9] Change initial value for free memory. Change initial value from 32767 (maximum value of a 16-bit signed integer) to __INT_MAX__ (compiler-defined maximum value for an int). --- CommandStation-EX.ino | 4 ++-- freeMemory.cpp | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CommandStation-EX.ino b/CommandStation-EX.ino index 4954223..6694244 100644 --- a/CommandStation-EX.ino +++ b/CommandStation-EX.ino @@ -123,8 +123,8 @@ void loop() LCDDisplay::loop(); // ignored if LCD not in use -// Report any decrease in memory (will automatically trigger on first call) - static int ramLowWatermark = 32767; // replaced on first loop + // Report any decrease in memory (will automatically trigger on first call) + static int ramLowWatermark = __INT_MAX__; // replaced on first loop int freeNow = minimumFreeMemory(); if (freeNow < ramLowWatermark) diff --git a/freeMemory.cpp b/freeMemory.cpp index f8d2a0b..f1ad965 100644 --- a/freeMemory.cpp +++ b/freeMemory.cpp @@ -32,7 +32,7 @@ extern char *__malloc_heap_start; #endif -static volatile int minimum_free_memory = 32767; +static volatile int minimum_free_memory = __INT_MAX__; static inline int freeMemory() { @@ -52,6 +52,7 @@ static inline int freeMemory() { // void updateMinimumFreeMemory(unsigned char extraBytes) { int spare = freeMemory()-extraBytes; + if (spare < 0) spare = 0; if (spare < minimum_free_memory) minimum_free_memory = spare; } From 0b3c0bfe9e71a09f0931d52ce705acb2093ad772 Mon Sep 17 00:00:00 2001 From: Neil McKechnie Date: Wed, 10 Mar 2021 10:33:42 +0000 Subject: [PATCH 9/9] Update freeMemory.cpp Add explanatory comment. --- freeMemory.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/freeMemory.cpp b/freeMemory.cpp index f1ad965..f640abe 100644 --- a/freeMemory.cpp +++ b/freeMemory.cpp @@ -49,6 +49,11 @@ static inline int freeMemory() { // Update low ram level. Allow for extra bytes to be specified // by estimation or inspection, that may be used by other // called subroutines. Must be called with interrupts disabled. +// +// Although __brkval may go up and down as heap memory is allocated +// and freed, this function records only the worst case encountered. +// So even if all of the heap is freed, the reported minimum free +// memory will not increase. // void updateMinimumFreeMemory(unsigned char extraBytes) { int spare = freeMemory()-extraBytes;