From 5389161162c3da2eae73484d6dab7a7c240e9e4e Mon Sep 17 00:00:00 2001 From: Juan Pablo Civile Date: Sat, 3 Aug 2019 23:01:16 -0300 Subject: [PATCH 1/6] routing: make log in findPath hot path use logClosure It generates heap allocations for it's params even if it won't end up using them. Reduces memory usage by 2mb --- routing/pathfind.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/routing/pathfind.go b/routing/pathfind.go index 33c452f0..46199e45 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -448,8 +448,11 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, fromVertex, toNode, amountToSend, ) - log.Tracef("path finding probability: fromnode=%v, tonode=%v, "+ - "probability=%v", fromVertex, toNode, edgeProbability) + log.Trace(newLogClosure(func() string { + return fmt.Sprintf("path finding probability: fromnode=%v,"+ + " tonode=%v, probability=%v", fromVertex, toNode, + edgeProbability) + })) // If the probability is zero, there is no point in trying. if edgeProbability == 0 { From 21417139365be32ebad0d64b436bcf205a73da6b Mon Sep 17 00:00:00 2001 From: Juan Pablo Civile Date: Tue, 20 Aug 2019 13:27:03 -0300 Subject: [PATCH 2/6] routing: avoid walking all nodes for path finding if we don't need to Calling `ForEachNode` hits the DB, and allocates and parses every node in the graph. Walking the channels also loads nodes from the DB, so this meant that each node was read/parsed/allocated several times per run. This reduces runtime by ~10ms and memory usage by ~4mb. --- routing/pathfind.go | 73 +++++++++++++++++---------------------------- 1 file changed, 27 insertions(+), 46 deletions(-) diff --git a/routing/pathfind.go b/routing/pathfind.go index 46199e45..cb753941 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -6,6 +6,7 @@ import ( "math" "time" + "github.com/btcsuite/btcd/btcec" "github.com/coreos/bbolt" "github.com/lightningnetwork/lnd/channeldb" @@ -324,56 +325,35 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, // traversal. nodeHeap := newDistanceHeap() - // For each node in the graph, we create an entry in the distance map - // for the node set with a distance of "infinity". graph.ForEachNode - // also returns the source node, so there is no need to add the source - // node explicitly. + // Holds the current best distance for a given node. distance := make(map[route.Vertex]nodeWithDist) - if err := g.graph.ForEachNode(tx, func(_ *bbolt.Tx, - node *channeldb.LightningNode) error { - // TODO(roasbeef): with larger graph can just use disk seeks - // with a visited map - vertex := route.Vertex(node.PubKeyBytes) - distance[vertex] = nodeWithDist{ - dist: infinity, - node: route.Vertex(node.PubKeyBytes), + + if r.DestPayloadTLV { + // Check if the target has TLV enabled + + targetKey, err := btcec.ParsePubKey(target[:], btcec.S256()) + if err != nil { + return nil, err } - // If we don't have any features for this node, then we can - // stop here. - if node.Features == nil || !r.DestPayloadTLV { - return nil + targetNode, err := g.graph.FetchLightningNode(targetKey) + if err != nil { + return nil, err } - // We only need to perform this check for the final node, so we - // can exit here if this isn't them. - if vertex != target { - return nil + if targetNode.Features != nil { + supportsTLV := targetNode.Features.HasFeature( + lnwire.TLVOnionPayloadOptional, + ) + if !supportsTLV { + return nil, fmt.Errorf("destination hop doesn't " + + "understand new TLV paylods") + } } - - // If we have any records for the final hop, then we'll check - // not to ensure that they are actually able to interpret them. - supportsTLV := node.Features.HasFeature( - lnwire.TLVOnionPayloadOptional, - ) - if !supportsTLV { - return fmt.Errorf("destination hop doesn't " + - "understand new TLV paylods") - } - - return nil - }); err != nil { - return nil, err } additionalEdgesWithSrc := make(map[route.Vertex][]*edgePolicyWithSource) for vertex, outgoingEdgePolicies := range g.additionalEdges { - // We'll also include all the nodes found within the additional - // edges that are not known to us yet in the distance map. - distance[vertex] = nodeWithDist{ - dist: infinity, - node: vertex, - } // Build reverse lookup to find incoming edges. Needed because // search is taken place from target to source. @@ -391,11 +371,11 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, } // We can't always assume that the end destination is publicly - // advertised to the network and included in the graph.ForEachNode call - // above, so we'll manually include the target node. The target node - // charges no fee. Distance is set to 0, because this is the starting - // point of the graph traversal. We are searching backwards to get the - // fees first time right and correctly match channel bandwidth. + // advertised to the network so we'll manually include the target node. + // The target node charges no fee. Distance is set to 0, because this + // is the starting point of the graph traversal. We are searching + // backwards to get the fees first time right and correctly match + // channel bandwidth. distance[target] = nodeWithDist{ dist: 0, weight: 0, @@ -551,7 +531,8 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, // route, return. It is important to also return if the distance // is equal, because otherwise the algorithm could run into an // endless loop. - if tempDist >= distance[fromVertex].dist { + current, ok := distance[fromVertex] + if ok && tempDist >= current.dist { return } From 3e60a2363267405ada711033909f0b629260717f Mon Sep 17 00:00:00 2001 From: Juan Pablo Civile Date: Fri, 23 Aug 2019 12:27:02 -0300 Subject: [PATCH 3/6] routing: pre-allocate the distance map to an estimated node count Pre-sizing these structures avoids a lot of map resizing, which causes copies and rehashing of entries. We mostly know that the map won't exceed that size, and it doesn't affect memory usage in any significant way. --- routing/heap.go | 5 +++-- routing/heap_test.go | 2 +- routing/pathfind.go | 33 +++++++++++++++++++-------------- 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/routing/heap.go b/routing/heap.go index 1cbbc5e5..a20f1d57 100644 --- a/routing/heap.go +++ b/routing/heap.go @@ -50,9 +50,10 @@ type distanceHeap struct { // newDistanceHeap initializes a new distance heap. This is required because // we must initialize the pubkeyIndices map for path-finding optimizations. -func newDistanceHeap() distanceHeap { +func newDistanceHeap(numNodes int) distanceHeap { distHeap := distanceHeap{ - pubkeyIndices: make(map[route.Vertex]int), + pubkeyIndices: make(map[route.Vertex]int, numNodes), + nodes: make([]nodeWithDist, 0, numNodes), } return distHeap diff --git a/routing/heap_test.go b/routing/heap_test.go index 4214e965..178bd9f1 100644 --- a/routing/heap_test.go +++ b/routing/heap_test.go @@ -17,7 +17,7 @@ func TestHeapOrdering(t *testing.T) { // First, create a blank heap, we'll use this to push on randomly // generated items. - nodeHeap := newDistanceHeap() + nodeHeap := newDistanceHeap(0) prand.Seed(1) diff --git a/routing/pathfind.go b/routing/pathfind.go index cb753941..e3e457cf 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -37,6 +37,11 @@ const ( // some effect with smaller time lock values. The value may need // tweaking and/or be made configurable in the future. RiskFactorBillionths = 15 + + // estimatedNodeCount is used to preallocate the path finding structures + // to avoid resizing and copies. It should be number on the same order as + // the number of active nodes in the network. + estimatedNodeCount = 10000 ) // pathFinder defines the interface of a path finding algorithm. @@ -320,14 +325,6 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, defer tx.Rollback() } - // First we'll initialize an empty heap which'll help us to quickly - // locate the next edge we should visit next during our graph - // traversal. - nodeHeap := newDistanceHeap() - - // Holds the current best distance for a given node. - distance := make(map[route.Vertex]nodeWithDist) - if r.DestPayloadTLV { // Check if the target has TLV enabled @@ -352,6 +349,19 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, } } + // First we'll initialize an empty heap which'll help us to quickly + // locate the next edge we should visit next during our graph + // traversal. + nodeHeap := newDistanceHeap(estimatedNodeCount) + + // Holds the current best distance for a given node. + distance := make(map[route.Vertex]nodeWithDist, estimatedNodeCount) + + // We'll use this map as a series of "next" hop pointers. So to get + // from `Vertex` to the target node, we'll take the edge that it's + // mapped to within `next`. + next := make(map[route.Vertex]*channeldb.ChannelEdgePolicy, estimatedNodeCount) + additionalEdgesWithSrc := make(map[route.Vertex][]*edgePolicyWithSource) for vertex, outgoingEdgePolicies := range g.additionalEdges { @@ -385,11 +395,6 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, probability: 1, } - // We'll use this map as a series of "next" hop pointers. So to get - // from `Vertex` to the target node, we'll take the edge that it's - // mapped to within `next`. - next := make(map[route.Vertex]*channeldb.ChannelEdgePolicy) - // processEdge is a helper closure that will be used to make sure edges // satisfy our specific requirements. processEdge := func(fromVertex route.Vertex, bandwidth lnwire.MilliSatoshi, @@ -659,7 +664,7 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, // Use the nextHop map to unravel the forward path from source to // target. - pathEdges := make([]*channeldb.ChannelEdgePolicy, 0, len(next)) + var pathEdges []*channeldb.ChannelEdgePolicy currentNode := source for currentNode != target { // TODO(roasbeef): assumes no cycles // Determine the next hop forward using the next map. From fc36df0e606cdeb98c0aca1a2831da36aa728bb4 Mon Sep 17 00:00:00 2001 From: Juan Pablo Civile Date: Sat, 24 Aug 2019 20:02:43 -0300 Subject: [PATCH 4/6] routing: avoid unneeded map access `processEdge` basically had 4 expensive operations: 3 map accesses and updating the heap. This removes one of those for a small performance gain. --- routing/pathfind.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/routing/pathfind.go b/routing/pathfind.go index e3e457cf..8a67ccf1 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -552,7 +552,7 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, // better than the current best known distance to this node. // The new better distance is recorded, and also our "next hop" // map is populated with this edge. - distance[fromVertex] = nodeWithDist{ + withDist := nodeWithDist{ dist: tempDist, weight: tempWeight, node: fromVertex, @@ -560,13 +560,14 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, incomingCltv: incomingCltv, probability: probability, } + distance[fromVertex] = withDist next[fromVertex] = edge - // Either push distance[fromVertex] onto the heap if the node + // Either push withDist onto the heap if the node // represented by fromVertex is not already on the heap OR adjust // its position within the heap via heap.Fix. - nodeHeap.PushOrFix(distance[fromVertex]) + nodeHeap.PushOrFix(withDist) } // TODO(roasbeef): also add path caching From df70095ad060511f0a015aa628b6fd17b684298e Mon Sep 17 00:00:00 2001 From: Juan Pablo Civile Date: Sat, 24 Aug 2019 22:52:13 -0300 Subject: [PATCH 5/6] routing: optimize path finding structures distance map now holds the edge the current path is coming from, removing the need for next map. Both distance map and distanceHeap now hold pointers instead of the full struct to reduce allocations and copies. Both these changes reduced path finding time by ~5% and memory usage by ~2mb. --- routing/heap.go | 13 +++++++++---- routing/heap_test.go | 8 ++++---- routing/pathfind.go | 38 +++++++++++++++----------------------- 3 files changed, 28 insertions(+), 31 deletions(-) diff --git a/routing/heap.go b/routing/heap.go index a20f1d57..62b3e3dd 100644 --- a/routing/heap.go +++ b/routing/heap.go @@ -3,6 +3,7 @@ package routing import ( "container/heap" + "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/routing/route" ) @@ -35,12 +36,15 @@ type nodeWithDist struct { // Includes the routing fees and a virtual cost factor to account for // time locks. weight int64 + + // nextHop is the edge this route comes from. + nextHop *channeldb.ChannelEdgePolicy } // distanceHeap is a min-distance heap that's used within our path finding // algorithm to keep track of the "closest" node to our source node. type distanceHeap struct { - nodes []nodeWithDist + nodes []*nodeWithDist // pubkeyIndices maps public keys of nodes to their respective index in // the heap. This is used as a way to avoid db lookups by using heap.Fix @@ -53,7 +57,7 @@ type distanceHeap struct { func newDistanceHeap(numNodes int) distanceHeap { distHeap := distanceHeap{ pubkeyIndices: make(map[route.Vertex]int, numNodes), - nodes: make([]nodeWithDist, 0, numNodes), + nodes: make([]*nodeWithDist, 0, numNodes), } return distHeap @@ -85,7 +89,7 @@ func (d *distanceHeap) Swap(i, j int) { // // NOTE: This is part of the heap.Interface implementation. func (d *distanceHeap) Push(x interface{}) { - n := x.(nodeWithDist) + n := x.(*nodeWithDist) d.nodes = append(d.nodes, n) d.pubkeyIndices[n.node] = len(d.nodes) - 1 } @@ -97,6 +101,7 @@ func (d *distanceHeap) Push(x interface{}) { func (d *distanceHeap) Pop() interface{} { n := len(d.nodes) x := d.nodes[n-1] + d.nodes[n-1] = nil d.nodes = d.nodes[0 : n-1] delete(d.pubkeyIndices, x.node) return x @@ -107,7 +112,7 @@ func (d *distanceHeap) Pop() interface{} { // modify its position and reorder the heap. If the vertex does not already // exist in the heap, then it is pushed onto the heap. Otherwise, we will end // up performing more db lookups on the same node in the pathfinding algorithm. -func (d *distanceHeap) PushOrFix(dist nodeWithDist) { +func (d *distanceHeap) PushOrFix(dist *nodeWithDist) { index, ok := d.pubkeyIndices[dist.node] if !ok { heap.Push(d, dist) diff --git a/routing/heap_test.go b/routing/heap_test.go index 178bd9f1..659653f7 100644 --- a/routing/heap_test.go +++ b/routing/heap_test.go @@ -24,12 +24,12 @@ func TestHeapOrdering(t *testing.T) { // Create 100 random entries adding them to the heap created above, but // also a list that we'll sort with the entries. const numEntries = 100 - sortedEntries := make([]nodeWithDist, 0, numEntries) + sortedEntries := make([]*nodeWithDist, 0, numEntries) for i := 0; i < numEntries; i++ { var pubKey [33]byte prand.Read(pubKey[:]) - entry := nodeWithDist{ + entry := &nodeWithDist{ node: route.Vertex(pubKey), dist: prand.Int63(), } @@ -55,9 +55,9 @@ func TestHeapOrdering(t *testing.T) { // One by one, pop of all the entries from the heap, they should come // out in sorted order. - var poppedEntries []nodeWithDist + var poppedEntries []*nodeWithDist for nodeHeap.Len() != 0 { - e := heap.Pop(&nodeHeap).(nodeWithDist) + e := heap.Pop(&nodeHeap).(*nodeWithDist) poppedEntries = append(poppedEntries, e) } diff --git a/routing/pathfind.go b/routing/pathfind.go index 8a67ccf1..4fa4d15f 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -355,12 +355,7 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, nodeHeap := newDistanceHeap(estimatedNodeCount) // Holds the current best distance for a given node. - distance := make(map[route.Vertex]nodeWithDist, estimatedNodeCount) - - // We'll use this map as a series of "next" hop pointers. So to get - // from `Vertex` to the target node, we'll take the edge that it's - // mapped to within `next`. - next := make(map[route.Vertex]*channeldb.ChannelEdgePolicy, estimatedNodeCount) + distance := make(map[route.Vertex]*nodeWithDist, estimatedNodeCount) additionalEdgesWithSrc := make(map[route.Vertex][]*edgePolicyWithSource) for vertex, outgoingEdgePolicies := range g.additionalEdges { @@ -386,7 +381,7 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, // is the starting point of the graph traversal. We are searching // backwards to get the fees first time right and correctly match // channel bandwidth. - distance[target] = nodeWithDist{ + distance[target] = &nodeWithDist{ dist: 0, weight: 0, node: target, @@ -552,18 +547,17 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, // better than the current best known distance to this node. // The new better distance is recorded, and also our "next hop" // map is populated with this edge. - withDist := nodeWithDist{ + withDist := &nodeWithDist{ dist: tempDist, weight: tempWeight, node: fromVertex, amountToReceive: amountToReceive, incomingCltv: incomingCltv, probability: probability, + nextHop: edge, } distance[fromVertex] = withDist - next[fromVertex] = edge - // Either push withDist onto the heap if the node // represented by fromVertex is not already on the heap OR adjust // its position within the heap via heap.Fix. @@ -582,7 +576,7 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, // Fetch the node within the smallest distance from our source // from the heap. - partialPath := heap.Pop(&nodeHeap).(nodeWithDist) + partialPath := heap.Pop(&nodeHeap).(*nodeWithDist) pivot := partialPath.node // If we've reached our source (or we don't have any incoming @@ -632,7 +626,7 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, // Check if this candidate node is better than what we // already have. - processEdge(route.Vertex(chanSource), edgeBandwidth, inEdge, pivot) + processEdge(chanSource, edgeBandwidth, inEdge, pivot) return nil } @@ -656,26 +650,24 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, } } - // If the source node isn't found in the next hop map, then a path - // doesn't exist, so we terminate in an error. - if _, ok := next[source]; !ok { - return nil, newErrf(ErrNoPathFound, "unable to find a path to "+ - "destination") - } - - // Use the nextHop map to unravel the forward path from source to + // Use the distance map to unravel the forward path from source to // target. var pathEdges []*channeldb.ChannelEdgePolicy currentNode := source for currentNode != target { // TODO(roasbeef): assumes no cycles // Determine the next hop forward using the next map. - nextNode := next[currentNode] + currentNodeWithDist, ok := distance[currentNode] + if !ok { + // If the node doesnt have a next hop it means we didn't find a path. + return nil, newErrf(ErrNoPathFound, "unable to find a "+ + "path to destination") + } // Add the next hop to the list of path edges. - pathEdges = append(pathEdges, nextNode) + pathEdges = append(pathEdges, currentNodeWithDist.nextHop) // Advance current node. - currentNode = route.Vertex(nextNode.Node.PubKeyBytes) + currentNode = currentNodeWithDist.nextHop.Node.PubKeyBytes } // The route is invalid if it spans more than 20 hops. The current From 818c302d46381251fbb916884dc833adec816485 Mon Sep 17 00:00:00 2001 From: Juan Pablo Civile Date: Sun, 25 Aug 2019 01:24:06 -0300 Subject: [PATCH 6/6] routing: use nodeWithDist instead of vertex to avoid map access The same nodeWithDist was fetched from the map for every channel it has. This struct is not mutated so it can be fetched before and reused. --- routing/pathfind.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/routing/pathfind.go b/routing/pathfind.go index 4fa4d15f..743adad1 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -393,7 +393,7 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, // processEdge is a helper closure that will be used to make sure edges // satisfy our specific requirements. processEdge := func(fromVertex route.Vertex, bandwidth lnwire.MilliSatoshi, - edge *channeldb.ChannelEdgePolicy, toNode route.Vertex) { + edge *channeldb.ChannelEdgePolicy, toNodeDist *nodeWithDist) { edgesExpanded++ @@ -420,17 +420,16 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, // Calculate amount that the candidate node would have to sent // out. - toNodeDist := distance[toNode] amountToSend := toNodeDist.amountToReceive // Request the success probability for this edge. edgeProbability := r.ProbabilitySource( - fromVertex, toNode, amountToSend, + fromVertex, toNodeDist.node, amountToSend, ) log.Trace(newLogClosure(func() string { return fmt.Sprintf("path finding probability: fromnode=%v,"+ - " tonode=%v, probability=%v", fromVertex, toNode, + " tonode=%v, probability=%v", fromVertex, toNodeDist.node, edgeProbability) })) @@ -626,7 +625,7 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, // Check if this candidate node is better than what we // already have. - processEdge(chanSource, edgeBandwidth, inEdge, pivot) + processEdge(chanSource, edgeBandwidth, inEdge, partialPath) return nil } @@ -646,7 +645,7 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, bandWidth := partialPath.amountToReceive for _, reverseEdge := range additionalEdgesWithSrc[pivot] { processEdge(reverseEdge.sourceNode, bandWidth, - reverseEdge.edge, pivot) + reverseEdge.edge, partialPath) } }