From 32a1a3d1c03de7c7a036ce7eefc0ac0599325bb4 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Mon, 15 Aug 2022 11:22:28 -0700 Subject: [PATCH] util/deephash: avoid variadic argument for Update (#5372) Hashing []any is slow since hashing of interfaces is slow. Hashing of interfaces is slow since we pessimistically assume that cycles can occur through them and start cycle tracking. Drop the variadic signature of Update and fix callers to pass in an anonymous struct so that we are hashing concrete types near the root of the value tree. Signed-off-by: Joe Tsai Signed-off-by: Joe Tsai --- ipn/ipnlocal/local.go | 10 +++++++++- util/deephash/deephash.go | 11 +++++------ wgengine/userspace.go | 13 ++++++++++--- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 1b5ab2cae..e866c1e8b 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -1180,7 +1180,15 @@ func (b *LocalBackend) updateFilterLocked(netMap *netmap.NetworkMap, prefs *ipn. sshPol = *netMap.SSHPolicy } - changed := deephash.Update(&b.filterHash, haveNetmap, addrs, packetFilter, localNets.Ranges(), logNets.Ranges(), shieldsUp, sshPol) + changed := deephash.Update(&b.filterHash, &struct { + HaveNetmap bool + Addrs []netip.Prefix + FilterMatch []filter.Match + LocalNets []netipx.IPRange + LogNets []netipx.IPRange + ShieldsUp bool + SSHPolicy tailcfg.SSHPolicy + }{haveNetmap, addrs, packetFilter, localNets.Ranges(), logNets.Ranges(), shieldsUp, sshPol}) if !changed { return } diff --git a/util/deephash/deephash.go b/util/deephash/deephash.go index b6019a8c5..15a68ec67 100644 --- a/util/deephash/deephash.go +++ b/util/deephash/deephash.go @@ -188,14 +188,13 @@ func HasherForType[T any]() func(T) Sum { } // Update sets last to the hash of v and reports whether its value changed. -func Update(last *Sum, v ...any) (changed bool) { +func Update(last *Sum, v any) (changed bool) { sum := Hash(v) - if sum == *last { - // unchanged. - return false + changed = sum != *last + if changed { + *last = sum } - *last = sum - return true + return changed } var appenderToType = reflect.TypeOf((*appenderTo)(nil)).Elem() diff --git a/wgengine/userspace.go b/wgengine/userspace.go index 2ec61c359..8b590f8bc 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -704,8 +704,12 @@ func (e *userspaceEngine) maybeReconfigWireguardLocked(discoChanged map[key.Node } e.lastNMinPeers = len(min.Peers) - if !deephash.Update(&e.lastEngineSigTrim, &min, trimmedNodes, trackNodes, trackIPs) { - // No changes + if changed := deephash.Update(&e.lastEngineSigTrim, &struct { + WGConfig *wgcfg.Config + TrimmedNodes map[key.NodePublic]bool + TrackNodes []key.NodePublic + TrackIPs []netip.Addr + }{&min, trimmedNodes, trackNodes, trackIPs}); !changed { return nil } @@ -856,7 +860,10 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config, isSubnetRouterChanged := isSubnetRouter != e.lastIsSubnetRouter engineChanged := deephash.Update(&e.lastEngineSigFull, cfg) - routerChanged := deephash.Update(&e.lastRouterSig, routerCfg, dnsCfg) + routerChanged := deephash.Update(&e.lastRouterSig, &struct { + RouterConfig *router.Config + DNSConfig *dns.Config + }{routerCfg, dnsCfg}) if !engineChanged && !routerChanged && listenPort == e.magicConn.LocalPort() && !isSubnetRouterChanged { return ErrNoChanges }