state: skip route comparison for writes that leave routes alone

Reuse immutable route inputs when classifying NodeStore writes.
Only derive routes when an announcement or approval changes.

hscontrol/state/node_store.go

@@ -259,6 +259,8 @@ func (s *NodeStore) PutNode(n types.Node) types.NodeView {
 }
 
 // UpdateNodeFunc is a function type that takes a pointer to a [types.Node] and modifies it.
+// Referenced data, including Hostinfo and slices, is shared with published
+// snapshots. Replace it instead of mutating it in place.
 type UpdateNodeFunc func(n *types.Node)
 
 // UpdateNode applies a function to modify a specific node in the
@@ -537,7 +539,7 @@ func (s *NodeStore) applyBatch(batch []work) {
 				}
 
 				oldGivenName := n.GivenName
-				pre := n.Clone()
+				pre := n
 				fn(&n)
 
 				if n.GivenName != oldGivenName {
@@ -550,7 +552,7 @@ func (s *NodeStore) applyBatch(batch []work) {
 					w.pre[id] = pre.View()
 				}
 
-				relation, election := updateChanges(pre, &n)
+				relation, election := updateChanges(&pre, &n)
 				relationChanged = relationChanged || relation
 				electionChanged = electionChanged || election
 			}

hscontrol/state/node_store_test.go

@@ -1682,6 +1682,53 @@ func TestUpdateNodeDiff(t *testing.T) {
 	assert.False(t, before.Valid())
 }
 
+func TestUpdateNodeDiffSharesUnchangedRoutes(t *testing.T) {
+	node := createTestNode(1, 1, "user1", "node1")
+	peer := createTestNode(2, 2, "user2", "node2")
+	announced := make([]netip.Prefix, 2048)
+
+	ip := netip.MustParseAddr("10.0.0.0")
+	for i := range announced {
+		announced[i] = netip.PrefixFrom(ip, 32)
+		ip = ip.Next()
+	}
+
+	node.Hostinfo = &tailcfg.Hostinfo{RoutableIPs: announced}
+	node.ApprovedRoutes = []netip.Prefix{announced[0]}
+	lastSeen := time.Now().Add(-time.Minute)
+	node.LastSeen = &lastSeen
+
+	var peerBuilds atomic.Int64
+
+	store := NewNodeStore(types.Nodes{&node, &peer},
+		func(nodes []types.NodeView) map[types.NodeID][]types.NodeID {
+			peerBuilds.Add(1)
+
+			return allowAllPeersFunc(nodes)
+		}, TestBatchSize, TestBatchTimeout)
+	store.Start()
+
+	defer store.Stop()
+
+	peerBuilds.Store(0)
+
+	published := store.data.Load()
+	now := time.Now()
+	before, after, ok := store.UpdateNodeDiff(1, func(n *types.Node) {
+		n.LastSeen = &now
+	})
+	require.True(t, ok)
+	assert.Same(t, &before.AnnouncedRoutes()[0], &after.AnnouncedRoutes()[0])
+	assert.Same(t, &published.nodesByID[1].Hostinfo.RoutableIPs[0],
+		&after.AnnouncedRoutes()[0])
+	assert.Equal(t, lastSeen, before.LastSeen().Get())
+	assert.Equal(t, lastSeen, *published.nodesByID[1].LastSeen)
+	assert.Equal(t, now, after.LastSeen().Get())
+	assert.Zero(t, peerBuilds.Load())
+	assert.Same(t, &published.peersByNode[1][0], &store.data.Load().peersByNode[1][0])
+	assert.Equal(t, now, store.ListPeers(2).At(0).LastSeen().Get())
+}
+
 // TestListPeersExcludesSelf proves a node is never returned among its own
 // peers, on both the snapshot path and the explicit peer-ID path.
 //
@@ -2032,15 +2079,20 @@ func benchNodes(n int) ([]types.User, types.Nodes) {
 // (lastseen) should cost far fewer builds than a relation-changing one
 // (route, tag).
 func BenchmarkNodeStoreWrite(b *testing.B) {
-	users, nodes := benchNodes(617)
-
 	for _, kind := range []struct {
-		name   string
-		mutate func(i int, n *types.Node)
+		name      string
+		announced int
+		mutate    func(i int, n *types.Node)
 	}{
 		{name: "lastseen", mutate: func(_ int, n *types.Node) {
 			n.LastSeen = new(time.Now())
 		}},
+		{
+			name: "lastseen-many-routes", announced: 4096,
+			mutate: func(_ int, n *types.Node) {
+				n.LastSeen = new(time.Now())
+			},
+		},
 		{name: "route", mutate: func(i int, n *types.Node) {
 			subnet := netip.PrefixFrom(netip.AddrFrom4([4]byte{10, byte(200 + i%50), 0, 0}), 24) //nolint:gosec
 			n.Hostinfo = &tailcfg.Hostinfo{RoutableIPs: []netip.Prefix{subnet}}
@@ -2056,6 +2108,17 @@ func BenchmarkNodeStoreWrite(b *testing.B) {
 			n.UserID, n.User = nil, nil
 		}},
 	} {
+		users, nodes := benchNodes(617)
+		if kind.announced > 0 {
+			nodes[0].Hostinfo.RoutableIPs = make([]netip.Prefix, kind.announced)
+
+			ip := netip.MustParseAddr("10.0.0.0")
+			for i := range kind.announced {
+				nodes[0].Hostinfo.RoutableIPs[i] = netip.PrefixFrom(ip, 32)
+				ip = ip.Next()
+			}
+		}
+
 		b.Run(fmt.Sprintf("%s/n=%d", kind.name, len(nodes)), func(b *testing.B) {
 			var calls atomic.Int64
 

hscontrol/state/state.go

@@ -3781,7 +3781,7 @@ func (s *State) UpdateNodeFromMapRequest(id types.NodeID, req tailcfg.MapRequest
 		// update that shrinks the announced set can drop the node
 		// out of HA candidacy without touching ApprovedRoutes.
 		// Clear any stale Unhealthy bit in that case.
-		if len(currentNode.AllApprovedRoutes()) == 0 {
+		if currentNode.Unhealthy && len(currentNode.AllApprovedRoutes()) == 0 {
 			currentNode.Unhealthy = false
 		}
 	})

hscontrol/types/node.go

@@ -1286,6 +1286,15 @@ func equalUnordered[E comparable](a, b []E, cmp func(E, E) int) bool {
 	return slices.Equal(ac, bc)
 }
 
+// Unchanged snapshots share immutable slices, so pointer equality avoids a scan.
+func sameSlice[E comparable](a, b []E) bool {
+	if len(a) != len(b) {
+		return false
+	}
+
+	return len(a) == 0 || &a[0] == &b[0] || slices.Equal(a, b)
+}
+
 // HasPolicyChange reports whether the node has changes that affect
 // policy evaluation. Includes approved subnet routes because they act
 // as source identity in [Node.CanAccess] for subnet-to-subnet ACLs,
@@ -1311,6 +1320,11 @@ func (nv NodeView) HasPolicyChange(other NodeView) bool {
 		return true
 	}
 
+	if sameSlice(nv.AnnouncedRoutes(), other.AnnouncedRoutes()) &&
+		sameSlice(nv.ж.ApprovedRoutes, other.ж.ApprovedRoutes) {
+		return false
+	}
+
 	if !equalPrefixesUnordered(nv.SubnetRoutes(), other.SubnetRoutes()) {
 		return true
 	}

hscontrol/types/node_test.go

@@ -1078,6 +1078,54 @@ func TestNodeIsExpiredAt(t *testing.T) {
 	})
 }
 
+func TestHasPolicyChangeSharedRouteInputs(t *testing.T) {
+	subnet := netip.MustParsePrefix("10.0.0.0/24")
+	exit := netip.MustParsePrefix("::/0")
+	base := Node{
+		Hostinfo:       &tailcfg.Hostinfo{RoutableIPs: []netip.Prefix{subnet, exit}},
+		ApprovedRoutes: []netip.Prefix{subnet, exit},
+	}
+
+	for _, tt := range []struct {
+		name   string
+		mutate func(*Node)
+		want   bool
+	}{
+		{name: "shared slices", mutate: func(*Node) {}},
+		{name: "equal slices", mutate: func(n *Node) {
+			n.Hostinfo = n.Hostinfo.Clone()
+			n.ApprovedRoutes = []netip.Prefix{subnet, exit}
+		}},
+		{name: "reordered slices", mutate: func(n *Node) {
+			n.Hostinfo = &tailcfg.Hostinfo{RoutableIPs: []netip.Prefix{exit, subnet}}
+			n.ApprovedRoutes = []netip.Prefix{exit, subnet}
+		}},
+		{name: "approval removed", mutate: func(n *Node) {
+			n.ApprovedRoutes = []netip.Prefix{subnet}
+		}, want: true},
+		{name: "announcement removed", mutate: func(n *Node) {
+			n.Hostinfo = &tailcfg.Hostinfo{RoutableIPs: []netip.Prefix{subnet}}
+		}, want: true},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			other := base
+			tt.mutate(&other)
+			require.Equal(t, tt.want, base.View().HasPolicyChange(other.View()))
+		})
+	}
+
+	shared := base
+
+	var changed bool
+
+	allocs := testing.AllocsPerRun(10, func() {
+		changed = base.View().HasPolicyChange(shared.View())
+	})
+
+	require.False(t, changed)
+	require.Zero(t, allocs, "unchanged route inputs should not derive route lists")
+}
+
 func TestBoundHostinfo(t *testing.T) {
 	stock := func() *tailcfg.Hostinfo {
 		return &tailcfg.Hostinfo{