Merged this now. It’s a safe change with no behavioural impact, and it removes unnecessary work in the hot paths when DEBUG logging is off. Happy to revisit if we want to standardise on lazy formatting later, but this gives us an immediate win.
Problem
-------
update_packet_metrics() called rrdtool.info() (cached for 5 s) to get the
RRD's last_update timestamp. rrdtool.info() returns a massive Python dict:
17 data sources × 5 RRAs × ~8 fields each = ~700+ dict entries per call.
tracemalloc showed +10696 new allocations / +251 KB at this exact line,
flagged as "Investigate" in the memory diagnostics dashboard.
The rrdtool.info() approach was also unnecessarily complex: it required a
5-second secondary cache, a _pending_rrd_update buffer, and two extra
instance attributes — all to answer one question ("did we already write
this period?") that we can answer ourselves with a single integer.
Fix
---
Replace _last_rrd_info_cache / _last_rrd_info_time / _pending_rrd_update
with a single self._last_rrd_update: int = 0 that stores the timestamp of
the last successful rrdtool.update() call. The throttle check becomes:
if timestamp <= self._last_rrd_update:
return
On success: self._last_rrd_update = timestamp
Zero dict allocations per call. The only downside vs rrdtool.info() is
that _last_rrd_update resets to 0 on process restart, meaning the first
packet after a restart always triggers a write — correct behaviour.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Problem
-------
rrdtool.fetch() is a blocking C library call that reads 24 hours of RRD
data from disk. The dashboard can call get_data() on every page refresh.
On an SD card each fetch can cost several milliseconds of I/O, and because
the RRD step is 60 seconds the data cannot change more often than that —
any fetch within the same 60-second window returns identical data.
The combined-optimizations branch had a 60-second read cache; rightup's
batching refactor inadvertently removed it. This PR restores it.
Solution
--------
* Add self._get_data_cache: tuple = (0.0, None) to __init__
* In get_data(): set use_cache = (start_time is None and end_time is None)
- if use_cache and cache is < 60 s old: return cached result immediately
- after a successful live fetch with use_cache: store (now, result)
* Explicit start_time / end_time callers always bypass the cache so
fine-grained or historical queries are never stale
Why 60 s TTL?
The RRD step is 60 s, so the database cannot hold a newer sample until
the next step boundary. A 60-second cache is tight enough that the
dashboard always shows data ≤ one step stale, and loose enough that
a burst of refreshes costs one disk read instead of N.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Problem 1 — _recent_drops: the list was evicted with pop(0), which is an
O(n) memmove every time a drop is recorded. With maxlen=20 this is
negligible today, but pop(0) on a list is always O(n) and the pattern is
worth eliminating.
Problem 2 — _known_neighbors cap: the eviction path did
set(list(self._known_neighbors)[500:])
which first materialises the entire set as a list (O(n) allocation) before
slicing. itertools.islice works directly on the set iterator and only
allocates the 500 kept items, halving peak memory pressure during cleanup.
Changes:
* Import itertools (already absent from this file)
* Import deque from collections alongside OrderedDict
* self._recent_drops initialised as deque(maxlen=20); self._max_recent_drops
removed (maxlen is the single source of truth)
* Drop-recording block: rebuild deque from generator (preserves pubkey dedup
filter) then append — automatic eviction replaces the explicit pop(0) guard
* Known-neighbors cap: itertools.islice(self._known_neighbors, 500) replaces
list(self._known_neighbors)[500:]
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Python evaluates f-string arguments before calling logger.debug(), so in
production (INFO level) every debug log call in the hot path still paid the
cost of string formatting even though the output was discarded.
The most expensive sites are in __call__ (runs on every received packet):
- "RX packet: header=0x{...}, payload_len=..., path_len=..., rssi=..., snr=..."
- "Packet header=0x{...}, type=..., route=..."
And in _calculate_tx_delay (runs on every forwarded packet):
- "Route=FLOOD/DIRECT, len=...B, airtime=...ms, delay=...s"
- "Congestion detected, score=..., delay multiplier=..."
Plus transport code and local-TX debug logs (less frequent but same issue).
Fix: wrap each f-string logger.debug() call with
if logger.isEnabledFor(logging.DEBUG):
so the f-string is never constructed when debug logging is disabled.
logger.isEnabledFor() is a pure in-memory integer comparison — essentially
free at runtime. In production at INFO level this eliminates string
concatenation, attribute lookups (packet.header, len(packet.payload), etc.),
and format operations on every forwarded packet.
Eight call sites guarded; no logic changes.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Five targeted changes to sqlite_handler.py, all in the same file.
1. Thread-local persistent connections
_connect() previously opened a new sqlite3.connect() on every DB call and
ran journal_mode + busy_timeout PRAGMAs each time. On SD-card storage each
connection open involves file-system operations; each PRAGMA is a round-trip.
threading.local() now caches one connection per thread (write executor thread
+ event-loop/HTTP threads), eliminating per-call setup overhead.
2. PRAGMA synchronous=NORMAL
Default synchronous=FULL flushes WAL frames to disk after every transaction.
NORMAL flushes only at WAL checkpoints — safe for this workload (no data loss
beyond the current transaction on power failure) and significantly faster on
SD cards, which have slow fsync (5-20ms per flush).
3. Migration 8: UNIQUE index on companion_messages(companion_hash, packet_hash)
companion_push_message previously deduped via SELECT + INSERT (two statements,
two SD-card reads per message). The new UNIQUE index enables INSERT OR IGNORE,
replacing the round-trip with a single atomic statement.
4. Migration 9: UNIQUE index on adverts(pubkey)
Without this index store_advert's ON CONFLICT clause cannot fire and each
advert inserts a new row instead of updating the existing one — unbounded
table growth on busy meshes. The migration deduplicates existing rows
(keeping the most-recently-seen per pubkey) before adding the index.
5. Remove duplicate get_unsynced_count definition
The method was defined twice with the same signature. Python silently uses
the last definition; the first was dead code with reversed SQL parameter
binding order. Removed the first; added a note to the surviving definition.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Before this change, calculate_packet_hash() (SHA-256 + hex + upper) was called
3 times per forwarded packet and 4 times per dropped packet:
__call__ → pkt_hash_full = packet.calculate_packet_hash() #1
→ flood/direct_forward → is_duplicate → calculate_packet_hash() #2
→ flood/direct_forward → mark_seen → calculate_packet_hash() #3
(drop) → _get_drop_reason → is_duplicate → calculate_packet_hash() #4
pkt_hash_full was computed in __call__ but never threaded down into
process_packet, flood_forward, direct_forward, is_duplicate, or _get_drop_reason.
Each method recomputed it independently.
Fix: add optional packet_hash: Optional[str] = None to is_duplicate,
_get_drop_reason, flood_forward, direct_forward, and process_packet. Pass
pkt_hash_full from __call__ through the chain. Each method uses the provided
hash or falls back to computing it — preserving backward compatibility for
external callers (TraceHelper, etc.) that have no pre-computed hash.
Result: 1 SHA-256 computation per packet in the hot path regardless of whether
the packet is forwarded or dropped.
Also adds explicit INVARIANT docstrings to flood_forward, direct_forward, and
is_duplicate documenting that these methods must remain synchronous (no await).
The is_duplicate + mark_seen pair is atomic within the asyncio event loop; adding
an await between them would allow two concurrent tasks to both pass the duplicate
check for the same packet — forwarding it twice.
Docs: docs/pr_hash_once.md — problem analysis, call-chain diagram, per-method
diffs, quantification (~3-8 µs saved per packet), test plan (including hash-count
assertion), and proof that passing the original's hash to the deep-copied packet
is correct.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
