Skip to content

Conversation

@rcd
Copy link
Contributor

@rcd rcd commented Jan 31, 2026

PacketHistory::find() does a linear scan over the entire recent packet history array every time a packet comes in. This works, but it's O(n) and gets called on every received packet, sometimes twice in the same code path (NextHopRouter was calling wasRelayer() twice for the same packet record).

This PR adds an open-addressing hash table as a side index into the existing recentPackets array, making lookups O(1). The hash uses linear probing with a load factor ≤ 0.5 and backward-shift deletion instead of tombstones, so it holds up well under the constant eviction churn. The rest of the packet history logic (eviction, relayer tracking, merge) is untouched, the hash is just an index sitting alongside it.

Also added checkRelayers() to batch the two wasRelayer() calls in the ACK path into a single lookup, and gated the whole hash table implementation behind MESHTASTIC_EXCLUDE_PKT_HISTORY_HASH so it's excluded from minimal builds.

Added 48 unit tests covering the full PacketHistory surface.

Memory cost is ~2 KB for the default history size of 256 packets. Benchmarks on unit tests show this is about 100x faster on nRF52 and 40x faster on ESP32. That translates to minimal real world improvements, but does give a lot of headroom for routers handling much larger meshes and in future DoS prevention checks. There are also likely minimal battery improvements, but less time spent computing is more time spent sleeping, so it can't hurt.

🤝 Attestations

  • I have tested that my proposed changes behave as described.
  • I have tested that my proposed changes do not cause any obvious regressions on the following devices:
    • Heltec (Lora32) V3
    • LilyGo T-Deck
    • LilyGo T-Beam
    • RAK WisBlock 4631
    • Seeed Studio T-1000E tracker card
    • Other: Heltec t114 v2

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request adds an open-addressing hash table as a secondary index to PacketHistory, improving packet lookup performance from O(n) to O(1). The hash table uses linear probing with a 0.5 load factor and backward-shift deletion, complementing the existing recentPackets array without changing its eviction logic.

Changes:

  • Added hash table implementation with O(1) lookups for packet deduplication
  • Introduced checkRelayers() batch method to optimize multiple relayer checks
  • Added MESHTASTIC_EXCLUDE_PKT_HISTORY_HASH flag for minimal builds
  • Comprehensive test suite with 48 unit tests covering all PacketHistory functionality

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/mesh/PacketHistory.h Added hash table data structures and checkRelayers() method declaration
src/mesh/PacketHistory.cpp Implemented hash functions (insert, remove, rebuild, slot), updated find() to use hash index, integrated hash maintenance in insert()
src/meshUtils.h Added nextPowerOf2() utility for calculating hash table capacity
src/mesh/NextHopRouter.cpp Updated to use checkRelayers() for batch lookup optimization
src/configuration.h Added MESHTASTIC_EXCLUDE_PKT_HISTORY_HASH to minimal build exclusions
test/test_packet_history/test_main.cpp Comprehensive test suite with 48 tests covering initialization, deduplication, eviction, relayer tracking, merge logic, and hash stress tests

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Collaborator

@jp-bennett jp-bennett left a comment

Choose a reason for hiding this comment

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

I was ready to throw things when I saw it was 1000+ lines of new code, but very pleasantly surprised to see the majority are in the test cases. Code looks clean and the idea makes sense. The 2k hurts a bit, but other than that looks OK.
Edit: Maybe also needs to wait for 2.8? This one worries me less than the NodeDB sorting code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants