Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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