Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/MessageStore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "SPILock.h"
#include "SafeFile.h"
#include "gps/RTC.h"
#include "memory/MemAudit.h"
#include <cstring> // memcpy

#ifndef MESSAGE_TEXT_POOL_SIZE
Expand All @@ -28,8 +29,10 @@ static inline void resetMessagePool()
g_messagePool = static_cast<char *>(malloc(MESSAGE_TEXT_POOL_SIZE));
if (!g_messagePool) {
LOG_ERROR("MessageStore: Failed to allocate %d bytes for message pool", MESSAGE_TEXT_POOL_SIZE);
memaudit::set("msgstore", 0);
return;
}
memaudit::set("msgstore", MESSAGE_TEXT_POOL_SIZE);
}
g_poolWritePos = 0;
memset(g_messagePool, 0, MESSAGE_TEXT_POOL_SIZE);
Expand Down
4 changes: 4 additions & 0 deletions src/graphics/TFTDisplay.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "configuration.h"
#include "main.h"
#include "memory/MemAudit.h"
#if USE_TFTDISPLAY

#if ARCH_PORTDUINO
Expand Down Expand Up @@ -1228,6 +1229,7 @@ TFTDisplay::~TFTDisplay()
free(repaintChunkBuffer);
repaintChunkBuffer = nullptr;
}
memaudit::set("display", 0);
}

// Write the buffer to the display memory
Expand Down Expand Up @@ -1654,6 +1656,7 @@ bool TFTDisplay::connect()
LOG_ERROR("Not enough memory to create TFT line buffer\n");
return false;
}
memaudit::add("display", sizeof(uint16_t) * displayWidth);
}
if (this->repaintChunkBuffer == NULL) {
this->repaintChunkBuffer = (uint16_t *)malloc(sizeof(uint16_t) * displayWidth * kFullRepaintChunkRows);
Expand All @@ -1662,6 +1665,7 @@ bool TFTDisplay::connect()
LOG_ERROR("Not enough memory to create TFT repaint chunk buffer\n");
return false;
}
memaudit::add("display", sizeof(uint16_t) * displayWidth * kFullRepaintChunkRows);
}
return true;
}
Expand Down
4 changes: 4 additions & 0 deletions src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#include "detect/einkScan.h"
#include "graphics/Screen.h"
#include "main.h"
#include "memory/MemAudit.h"
#include "mesh/generated/meshtastic/config.pb.h"
#include "meshUtils.h"
#include "modules/Modules.h"
Expand Down Expand Up @@ -1151,6 +1152,9 @@ void setup()
LOG_DEBUG("Free PSRAM : %7d bytes", ESP.getFreePsram());
#endif

// Log the per-subsystem heap breakdown now that the big allocations are done
memaudit::logBreakdown("boot");

// We manually run this to update the NodeStatus
nodeDB->notifyObservers(true);
}
Expand Down
2 changes: 2 additions & 0 deletions src/memGet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
*/
#include "memGet.h"
#include "configuration.h"
#include "memory/MemAudit.h"

#if defined(MESHTASTIC_DYNAMIC_SBRK_HEAP)
#include <malloc.h>
Expand Down Expand Up @@ -118,4 +119,5 @@ void displayPercentHeapFree()
}
int percent = (int)((freeHeap * 100) / totalHeap);
LOG_INFO("Heap free: %d%% (%u/%u bytes)", percent, freeHeap, totalHeap);
memaudit::logBreakdown("heap"); // per-subsystem breakdown rides along with the periodic heap log
}
116 changes: 116 additions & 0 deletions src/memory/MemAudit.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
#include "MemAudit.h"

#if MESHTASTIC_MEM_AUDIT

#include "DebugConfiguration.h"
#include <atomic>
#include <stdio.h>
#include <string.h>

namespace memaudit
{

namespace
{

struct Entry {
std::atomic<const char *> tag; // registered literal; nullptr = free slot
std::atomic<int32_t> bytes;
};

// Static storage only - the accounting registry must never itself allocate.
// Zero-initialized (BSS), so it is usable from constructors of static objects.
Entry table[kMaxTags];

// Find the slot for a tag, registering it on first use.
// Returns nullptr for a null tag or when the table is full (update dropped).
Entry *findOrRegister(const char *tag)
{
if (!tag)
return nullptr;

// Fast path: same literal, pointer compare only. This is all the hot
// per-packet add() ever executes once the tag is registered.
size_t used = 0;
for (; used < kMaxTags; used++) {
const char *cur = table[used].tag.load(std::memory_order_acquire);
if (!cur)
break; // slots fill in order - first empty slot ends the table
if (cur == tag)
return &table[used];
}

// Slow path: same text from a different literal (duplicated across
// translation units, so not pointer-identical).
for (size_t i = 0; i < used; i++) {
if (strcmp(table[i].tag.load(std::memory_order_relaxed), tag) == 0)
return &table[i];
}

// First use: claim a free slot. compare_exchange keeps a registration race
// from double-claiming; the loser re-checks what the winner wrote.
for (size_t i = used; i < kMaxTags; i++) {
const char *expected = nullptr;
if (table[i].tag.compare_exchange_strong(expected, tag, std::memory_order_acq_rel))
return &table[i];
if (expected == tag || strcmp(expected, tag) == 0)
return &table[i];
}

return nullptr; // table full - bump kMaxTags if this ever happens
}

} // namespace

void add(const char *tag, int32_t delta)
{
Entry *e = findOrRegister(tag);
if (e)
e->bytes.fetch_add(delta, std::memory_order_relaxed);
}

void set(const char *tag, uint32_t bytes)
{
Entry *e = findOrRegister(tag);
if (e)
e->bytes.store((int32_t)bytes, std::memory_order_relaxed);
}

size_t snapshot(Tag *out, size_t max)
{
size_t n = 0;
for (size_t i = 0; i < kMaxTags && n < max; i++) {
const char *tag = table[i].tag.load(std::memory_order_acquire);
if (!tag)
break;
out[n].tag = tag;
out[n].bytes = table[i].bytes.load(std::memory_order_relaxed);
n++;
}
return n;
}

void logBreakdown(const char *when)
{
Tag rows[kMaxTags];
size_t n = snapshot(rows, kMaxTags);
if (n == 0)
return;

// Worst case per row: 16-char tag + '=' + "-2147483648" + ' ' = 29 bytes.
char line[kMaxTags * 30 + 1];
size_t pos = 0;
int32_t total = 0;
for (size_t i = 0; i < n; i++) {
int written = snprintf(line + pos, sizeof(line) - pos, "%s%s=%ld", pos ? " " : "", rows[i].tag, (long)rows[i].bytes);
if (written < 0 || pos + written >= sizeof(line))
break;
pos += written;
total += rows[i].bytes;
}
LOG_INFO("MemAudit[%s]: %s total=%ld", when ? when : "?", line, (long)total);
}

} // namespace memaudit

#endif // MESHTASTIC_MEM_AUDIT
77 changes: 77 additions & 0 deletions src/memory/MemAudit.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
#pragma once

#include "configuration.h"
#include <stddef.h>
#include <stdint.h>

// MemAudit: tiny per-subsystem heap accounting registry.
//
// Subsystems that own a large long-lived allocation report it here under a short
// tag ("nodedb", "pkthist", ...). logBreakdown() then prints one line, e.g.
// MemAudit[boot]: tmm=2500 warm=4000 pkthist=5824 nodedb=13440 total=25764
// so heap regressions in field reports self-diagnose from the serial log instead
// of needing a hand-built breakdown for every release.
//
// Tags must be string LITERALS (or otherwise immortal strings): the registry
// stores the pointer, compares by pointer first and falls back to strcmp for
// the same text duplicated across translation units.
//
// Concurrency: counters are 32-bit std::atomic accessed with relaxed ordering -
// on ARM Cortex-M aligned 32-bit loads/stores are single instructions and the
// update sites are low-rate, so add() stays a few instructions with no locks
// (the one hot path is the per-packet pool add). Registration claims a table
// slot with a compare-exchange, so first-use racing is safe too. Counts are
// best-effort diagnostics, not exact bookkeeping.
//
// Compiled out (no-op inline stubs, so call sites need no #ifdefs) when
// MESHTASTIC_MEM_AUDIT is 0 - the default on STM32WL, the tightest flash target.
#ifndef MESHTASTIC_MEM_AUDIT
#ifdef ARCH_STM32WL
#define MESHTASTIC_MEM_AUDIT 0
#else
#define MESHTASTIC_MEM_AUDIT 1
#endif
#endif

namespace memaudit
{

// Fixed registry capacity - updates for tags beyond this are dropped (bump if needed).
constexpr size_t kMaxTags = 16;

// One snapshot row, as returned by snapshot().
struct Tag {
const char *tag; // the literal passed to add()/set()
int32_t bytes; // current byte count for that subsystem
};

#if MESHTASTIC_MEM_AUDIT

// Adjust a subsystem's byte count (registers the tag on first use). Safe from
// concurrent threads; this is the form to use on per-object alloc/free paths.
void add(const char *tag, int32_t delta);

// Set a subsystem's byte count outright - for one-shot pool/table allocations
// where the total is known (use 0 on free or allocation failure).
void set(const char *tag, uint32_t bytes);

// Copy up to max registered tags into out; returns the number written.
size_t snapshot(Tag *out, size_t max);

// Log the whole table as a single LOG_INFO line, labeled with `when` ("boot", ...).
void logBreakdown(const char *when);

#else

// No-op stubs so call sites compile away without #ifdefs.
inline void add(const char *, int32_t) {}
inline void set(const char *, uint32_t) {}
inline size_t snapshot(Tag *, size_t)
{
return 0;
}
inline void logBreakdown(const char *) {}

#endif // MESHTASTIC_MEM_AUDIT

} // namespace memaudit
21 changes: 19 additions & 2 deletions src/mesh/MemoryPool.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,15 @@

#include "PointerQueue.h"
#include "configuration.h" // For LOG_WARN, LOG_DEBUG, LOG_HEAP
#include "memory/MemAudit.h"

template <class T> class Allocator
{

public:
Allocator() : deleter([this](T *p) { this->release(p); }) {}
/// Optional memaudit tag: when set, live objects from this allocator are
/// reported under it (+/- sizeof(T) per alloc/release).
explicit Allocator(const char *auditTag = nullptr) : deleter([this](T *p) { this->release(p); }), auditTag(auditTag) {}
virtual ~Allocator() {}

/// Return a queable object which has been prefilled with zeros. Return nullptr if no buffer is available
Expand Down Expand Up @@ -73,9 +76,17 @@ template <class T> class Allocator
// Alloc some storage
virtual T *alloc(TickType_t maxWait) = 0;

// Report a live-object delta to memaudit (no-op when untagged)
void auditAdd(int32_t delta)
{
if (auditTag)
memaudit::add(auditTag, delta);
}

private:
// std::unique_ptr Deleter function; calls release().
const std::function<void(T *)> deleter;
const char *auditTag; // memaudit tag, or nullptr for untracked pools
};

/**
Expand All @@ -84,6 +95,8 @@ template <class T> class Allocator
template <class T> class MemoryDynamic : public Allocator<T>
{
public:
explicit MemoryDynamic(const char *auditTag = nullptr) : Allocator<T>(auditTag) {}

/// Return a buffer for use by others
virtual void release(T *p) override
{
Expand All @@ -92,6 +105,7 @@ template <class T> class MemoryDynamic : public Allocator<T>

LOG_HEAP("Freeing 0x%x", p);

this->auditAdd(-(int32_t)sizeof(T));
free(p);
}

Expand All @@ -101,6 +115,7 @@ template <class T> class MemoryDynamic : public Allocator<T>
{
T *p = (T *)malloc(sizeof(T));
assert(p);
this->auditAdd((int32_t)sizeof(T));
return p;
}
};
Expand All @@ -115,7 +130,7 @@ template <class T, int MaxSize> class MemoryPool : public Allocator<T>
bool used[MaxSize];

public:
MemoryPool() : pool{}, used{}
explicit MemoryPool(const char *auditTag = nullptr) : Allocator<T>(auditTag), pool{}, used{}
{
// Arrays are now zero-initialized by member initializer list
// pool array: all elements are default-constructed (zero for POD types)
Expand All @@ -135,6 +150,7 @@ template <class T, int MaxSize> class MemoryPool : public Allocator<T>
if (index >= 0 && index < MaxSize) {
assert(used[index]); // Should be marked as used
used[index] = false;
this->auditAdd(-(int32_t)sizeof(T));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
sed -n '130,170p' src/mesh/MemoryPool.h

Repository: meshtastic/firmware

Length of output: 1578


🏁 Script executed:

#!/bin/bash
sed -n '170,220p' src/mesh/MemoryPool.h

Repository: meshtastic/firmware

Length of output: 389


Guard release() against already-free slots before adjusting audit state. alloc() already skips auditAdd() when the pool is full, but release() only checks range/null and then decrements after an assert(used[index]). In release builds, an already-free slot would still change the counter and can drift over time.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/mesh/MemoryPool.h` at line 153, Guard MemoryPool::release() against
already-free slots before changing audit state. In the release path, after the
existing range/null checks and before auditAdd on the freed slot, make sure the
slot is still marked used in the used[] tracking for MemoryPool<T>; if it is
already free, return without touching the audit counter. Update the logic around
the assert(used[index]) check so release() does not decrement audit state in
release builds for double-frees.

LOG_HEAP("Released static pool item %d at 0x%x", index, p);
} else {
LOG_WARN("Pointer 0x%x not from our pool!", p);
Expand All @@ -149,6 +165,7 @@ template <class T, int MaxSize> class MemoryPool : public Allocator<T>
for (int i = 0; i < MaxSize; i++) {
if (!used[i]) {
used[i] = true;
this->auditAdd((int32_t)sizeof(T));
LOG_HEAP("Allocated static pool item %d at 0x%x", i, &pool[i]);
return &pool[i];
}
Expand Down
Loading
Loading