-
-
Notifications
You must be signed in to change notification settings - Fork 2k
Hash table index for O(1) packet history lookups #9499
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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_HASHflag 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>
There was a problem hiding this 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.
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