From 8df7fd2cd45e426aba1e91045f60f0d31c8c48f8 Mon Sep 17 00:00:00 2001 From: Lukas Vogel Date: Mon, 2 Sep 2024 11:20:49 +0200 Subject: [PATCH] daemon/combinator: memoize calculated IDs Do not recalculate IDs frequently. Also re-use buffer for fingerprint calculation. Use slices package for sorting in the cominator. --- private/path/combinator/combinator.go | 48 ++++++++---- private/path/combinator/combinator_test.go | 1 + private/path/combinator/export_test.go | 2 + private/path/combinator/graph.go | 90 ++++++++++------------ private/revcache/util.go | 3 - 5 files changed, 77 insertions(+), 67 deletions(-) diff --git a/private/path/combinator/combinator.go b/private/path/combinator/combinator.go index 14b15d8ba9..a1d7dcc4fd 100644 --- a/private/path/combinator/combinator.go +++ b/private/path/combinator/combinator.go @@ -23,7 +23,8 @@ package combinator import ( "crypto/sha256" "encoding/binary" - "sort" + "hash" + "slices" "github.com/scionproto/scion/pkg/addr" seg "github.com/scionproto/scion/pkg/segment" @@ -61,8 +62,9 @@ func Combine(src, dst addr.IA, ups, cores, downs []*seg.PathSegment, solutions := newDMG(ups, cores, downs).GetPaths(vertexFromIA(src), vertexFromIA(dst)) paths := make([]Path, len(solutions)) + st := newHashState() for i, solution := range solutions { - paths[i] = solution.Path() + paths[i] = solution.Path(st) } paths = filterLongPaths(paths) if !findAllIdentical { @@ -72,10 +74,11 @@ func Combine(src, dst addr.IA, ups, cores, downs []*seg.PathSegment, } type Path struct { - Dst addr.IA - SCIONPath path.SCION - Metadata snet.PathMetadata - Weight int // XXX(matzf): unused, drop this? + Dst addr.IA + SCIONPath path.SCION + Metadata snet.PathMetadata + Weight int // XXX(matzf): unused, drop this? + Fingerprint snet.PathFingerprint } // filterLongPaths returns a new slice containing only those paths that do not @@ -112,10 +115,9 @@ func filterDuplicates(paths []Path) []Path { // unique path interface sequence (== fingerprint). uniquePaths := make(map[snet.PathFingerprint]int) for i, p := range paths { - key := fingerprint(p.Metadata.Interfaces) - prev, dupe := uniquePaths[key] + prev, dupe := uniquePaths[p.Fingerprint] if !dupe || p.Metadata.Expiry.After(paths[prev].Metadata.Expiry) { - uniquePaths[key] = i + uniquePaths[p.Fingerprint] = i } } @@ -123,7 +125,7 @@ func filterDuplicates(paths []Path) []Path { for _, idx := range uniquePaths { toKeep = append(toKeep, idx) } - sort.Ints(toKeep) + slices.Sort(toKeep) filtered := make([]Path, 0, len(toKeep)) for _, i := range toKeep { filtered = append(filtered, paths[i]) @@ -135,8 +137,9 @@ func filterDuplicates(paths []Path) []Path { // ASes and BRs, i.e. by its PathInterfaces. // XXX(matzf): copied from snet.Fingerprint. Perhaps snet.Fingerprint could be adapted to // take []snet.PathInterface directly. -func fingerprint(interfaces []snet.PathInterface) snet.PathFingerprint { - h := sha256.New() +func fingerprint(interfaces []snet.PathInterface, st hashState) snet.PathFingerprint { + h := st.hash + h.Reset() for _, intf := range interfaces { if err := binary.Write(h, binary.BigEndian, intf.IA); err != nil { panic(err) @@ -145,5 +148,24 @@ func fingerprint(interfaces []snet.PathInterface) snet.PathFingerprint { panic(err) } } - return snet.PathFingerprint(h.Sum(nil)) + // convert the snet.PathFingerprint, this is safe because the underlying + // type is string. + return snet.PathFingerprint(h.Sum(st.buf[:0])) +} + +func checkUnderlyingType[S ~string](_ S) bool { return true } + +// This check ensures that the underlying type of the PathFingerprint +// is string. This is required to create a copy of the buffer. If it were +// to be moved to a []byte, we need to change the code to create +// a proper copy. +var _ = checkUnderlyingType(snet.PathFingerprint("")) + +type hashState struct { + hash hash.Hash + buf []byte +} + +func newHashState() hashState { + return hashState{hash: sha256.New(), buf: make([]byte, 0, sha256.Size)} } diff --git a/private/path/combinator/combinator_test.go b/private/path/combinator/combinator_test.go index f65e629986..f9aced5828 100644 --- a/private/path/combinator/combinator_test.go +++ b/private/path/combinator/combinator_test.go @@ -598,6 +598,7 @@ func TestFilterDuplicates(t *testing.T) { Interfaces: interfaces, Expiry: expiry, }, + Fingerprint: combinator.Fingerprint(interfaces, combinator.NewHashState()), } } diff --git a/private/path/combinator/export_test.go b/private/path/combinator/export_test.go index 256b10af8c..4d374768e0 100644 --- a/private/path/combinator/export_test.go +++ b/private/path/combinator/export_test.go @@ -16,4 +16,6 @@ package combinator var ( FilterDuplicates = filterDuplicates + Fingerprint = fingerprint + NewHashState = newHashState ) diff --git a/private/path/combinator/graph.go b/private/path/combinator/graph.go index 2531c77b78..090e11ad17 100644 --- a/private/path/combinator/graph.go +++ b/private/path/combinator/graph.go @@ -16,10 +16,11 @@ package combinator import ( "bytes" + "cmp" "encoding/binary" "fmt" "math" - "sort" + "slices" "time" "github.com/scionproto/scion/pkg/addr" @@ -193,9 +194,9 @@ func (g *dmg) AddEdge(src, dst vertex, segment *inputSegment, e *edge) { } // GetPaths returns all the paths from src to dst, sorted according to weight. -func (g *dmg) GetPaths(src, dst vertex) pathSolutionList { - var solutions pathSolutionList - queue := pathSolutionList{&pathSolution{currentVertex: src}} +func (g *dmg) GetPaths(src, dst vertex) []*pathSolution { + var solutions []*pathSolution + queue := []*pathSolution{{currentVertex: src}} for len(queue) > 0 { currentPathSolution := queue[0] queue = queue[1:] @@ -234,7 +235,29 @@ func (g *dmg) GetPaths(src, dst vertex) pathSolutionList { } } } - sort.Sort(solutions) + slices.SortFunc(solutions, func(a, b *pathSolution) int { + d := cmp.Or( + cmp.Compare(a.cost, b.cost), + cmp.Compare(len(a.edges), len(b.edges)), + ) + if d != 0 { + return d + } + trailA, trailB := a.edges, b.edges + for ka := range trailA { + idA := trailA[ka].segment.ID() + idB := trailB[ka].segment.ID() + d := cmp.Or( + bytes.Compare(idA, idB), + cmp.Compare(trailA[ka].edge.Shortcut, trailB[ka].edge.Shortcut), + cmp.Compare(trailA[ka].edge.Peer, trailB[ka].edge.Peer), + ) + if d != 0 { + return d + } + } + return 0 + }) return solutions } @@ -247,6 +270,7 @@ func (g *dmg) GetPaths(src, dst vertex) pathSolutionList { type inputSegment struct { *seg.PathSegment Type proto.PathSegType + id []byte } // IsDownSeg returns true if the segment is a DownSegment. @@ -254,6 +278,13 @@ func (s *inputSegment) IsDownSeg() bool { return s.Type == proto.PathSegType_down } +func (s *inputSegment) ID() []byte { + if s.id == nil { + s.id = s.PathSegment.ID() + } + return s.id +} + // Vertex is a union-like type for the AS vertices and Peering link vertices in // a DMG that can be used as key in maps. type vertex struct { @@ -310,7 +341,7 @@ type pathSolution struct { // Path builds the forwarding path with metadata by extracting it from a path // between source and destination in the DMG. -func (solution *pathSolution) Path() Path { +func (solution *pathSolution) Path(hashState hashState) Path { mtu := ^uint16(0) var segments segmentList var epicPathAuths [][]byte @@ -427,7 +458,8 @@ func (solution *pathSolution) Path() Path { InternalHops: staticInfo.InternalHops, Notes: staticInfo.Notes, }, - Weight: solution.cost, + Weight: solution.cost, + Fingerprint: fingerprint(interfaces, hashState), } if authPHVF, authLHVF, ok := isEpicAvailable(epicPathAuths); ok { @@ -531,50 +563,6 @@ func calculateBeta(se *solutionEdge) uint16 { return beta } -// PathSolutionList is a sort.Interface implementation for a slice of solutions. -type pathSolutionList []*pathSolution - -func (sl pathSolutionList) Len() int { - return len(sl) -} - -// Less sorts according to the following priority list: -// - total path cost (number of hops) -// - number of segments -// - segmentIDs -// - shortcut index -// - peer entry index -func (sl pathSolutionList) Less(i, j int) bool { - if sl[i].cost != sl[j].cost { - return sl[i].cost < sl[j].cost - } - - trailI, trailJ := sl[i].edges, sl[j].edges - if len(trailI) != len(trailJ) { - return len(trailI) < len(trailJ) - } - - for ki := range trailI { - idI := trailI[ki].segment.ID() - idJ := trailJ[ki].segment.ID() - idcmp := bytes.Compare(idI, idJ) - if idcmp != 0 { - return idcmp == -1 - } - if trailI[ki].edge.Shortcut != trailJ[ki].edge.Shortcut { - return trailI[ki].edge.Shortcut < trailJ[ki].edge.Shortcut - } - if trailI[ki].edge.Peer != trailJ[ki].edge.Peer { - return trailI[ki].edge.Peer < trailJ[ki].edge.Peer - } - } - return false -} - -func (sl pathSolutionList) Swap(i, j int) { - sl[i], sl[j] = sl[j], sl[i] -} - // solutionEdge contains a graph edge and additional metadata required during // graph exploration. type solutionEdge struct { diff --git a/private/revcache/util.go b/private/revcache/util.go index 36f1b115c1..151b451d2a 100644 --- a/private/revcache/util.go +++ b/private/revcache/util.go @@ -44,9 +44,6 @@ func NoRevokedHopIntf(ctx context.Context, revCache RevCache, if err != nil || rev != nil { return false, err } - if rev != nil { - return false, nil - } } } return true, nil