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

@@ -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,18 +115,17 @@ 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
 		}
 	}
 
 	toKeep := make([]int, 0, len(uniquePaths))
 	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)}
 }

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()),
 		}
 	}
 

private/path/combinator/export_test.go

@@ -16,4 +16,6 @@ package combinator
 
 var (
 	FilterDuplicates = filterDuplicates
+	Fingerprint      = fingerprint
+	NewHashState     = newHashState
 )

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,13 +270,21 @@ 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.
 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 {

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