Skip to content

Conversation

@rcd
Copy link
Contributor

@rcd rcd commented Jan 29, 2026

This pull request modifies NodeDB to keep the meshNodes vector sorted by nodeNum so that it can be searched using a binary search O(log n).

The existing code prioritizes sorting for display: own node, favorites, last heard.

This display sorting forces a linear search O(n), which is usually not noticeable for small numbers of active nodes. As the number of nodes increases, this overhead can become significant for low power embedded boards.

In the existing code, the nodes are resorted every 5 seconds, regardless of if a user is viewing the node list or not. This pull request keeps a separate pointer vector sorted by the display order. The new vector is resorted only as needed for display, and then it is only sorted when something has changed, or every 5 seconds. There is no overhead for resorting if the display access methods are not called.

This pull request also makes meshNodes a private field and updates relevant accesses.

🤝 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 PR optimizes NodeDB lookups by maintaining the meshNodes vector sorted by NodeNum to enable O(log n) binary search instead of O(n) linear search. The existing display-order sorting (own node → favorites → last_heard) is moved to a separate lazy-built pointer vector that's only rebuilt when needed for display access.

Changes:

  • Refactored NodeDB to use binary search for node lookups by sorting meshNodes by NodeNum
  • Separated display ordering into displayNodes pointer vector with lazy rebuild on access
  • Added favoriteRouterLastBytes cache to optimize Router's shouldDecrementHopLimit check
  • Made meshNodes private and added appropriate accessor methods

Reviewed changes

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

Show a summary per file
File Description
src/mesh/NodeDB.h Added private meshNodes field, displayNodes vector, dirty flag, and new helper methods (rebuildDisplayOrder, sortByNodeNum, rebuildFavoriteRouterIndex)
src/mesh/NodeDB.cpp Implemented binary search in getMeshNode, insertion/removal maintaining sorted order, display order rebuilding, favorite router caching, and updated resetNodes/removeNodeByNum/cleanupMeshDB
src/modules/CannedMessageModule.cpp Simplified lookup by using getMeshNode directly instead of manual iteration
src/modules/AdminModule.cpp Replaced direct node modification with set_favorite method
src/mesh/Router.cpp Optimized shouldDecrementHopLimit using cached favoriteRouterLastBytes
src/modules/NodeInfoModule.cpp Updated to use getNumMeshNodes() instead of direct meshNodes access
src/mesh/PhoneAPI.cpp Changed STATE_SEND_OWN_NODEINFO to use getMeshNode instead of readNextMeshNode
src/graphics/niche/InkHUD/Applets/User/Heard/HeardApplet.cpp Updated to use readNextMeshNode and const correctness

@thebentern thebentern requested a review from fifieldt January 30, 2026 01:59
@Xaositek
Copy link
Contributor

The existing code prioritizes sorting for display: own node, favorites, last heard.

This is because that is how we leverage the node list, when looking at the node list on a service, that is the order they are displayed, so sorting against this order makes the most sense. We just filter based upon having a location for some of the GPS frame. While ordering the order every 5 seconds maybe can be backed off a bit (maybe just sort on change?) I don't understand the perform game of going to node number order to not use the data in that fashion.

@rcd
Copy link
Contributor Author

rcd commented Jan 30, 2026

Re: Xaositek

The display order (own node > favorites > last heard) is fully preserved. The displayNodes pointer vector still provides exactly that, and it's what getMeshNodeByIndex() returns. Nothing changes about how the UI sees or iterates nodes.

The nodeNum ordering applies to meshNodes, the internal structure used for lookups by node ID (e.g., finding a sender's node info when a packet arrives). That's currently a linear scan, and nodeNum sorted order gives us binary search instead.

So really it's a separation of concerns: display code sees the same ordering it always has, internal lookups get faster.

This is a relatively minor optimization in the realm of a largely IO bound mesh node. However, it can be made with little impact to the existing code structure and with full backward compatibility for the existing network. As long as the complexity or code size doesn't increase too much, small wins are still wins. The less time a node can spend doing CPU bound tasks, the more time it can spend sleeping and conserving battery.

On the display sort throttle, it could potentially be eliminated completely since the node list is now marked dirty when it is updated. It is probably best to keep the throttle/holddown since a mesh with lots of active nodes could be updated quite frequently and the end user probably doesn't care about or actually desire the list sorted in real time.

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 don't see anything I immediately hate. I do remember introducing a very mysterious crash the last time I worked with this sorting code, so we want to test this pretty thoroughly before shipping it. I know this is a pain, but maybe wait for the 2.8 releases?

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.

4 participants