Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xdr and exp/orderbook: Reduce path search allocations #4105

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 37 additions & 9 deletions exp/orderbook/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,7 @@ func (graph *OrderBookGraph) FindPaths(
maxAssetsPerPath int,
includePools bool,
) ([]Path, uint32, error) {

destinationAssetString := destinationAsset.String()
sourceAssetsMap := make(map[int32]xdr.Int64, len(sourceAssets))
for i, sourceAsset := range sourceAssets {
Expand Down Expand Up @@ -406,6 +407,23 @@ func (graph *OrderBookGraph) FindPaths(
return paths, lastLedger, err
}

type sortablePaths struct {
paths []Path
less func(paths []Path, i, j int) bool
}

func (s sortablePaths) Swap(i, j int) {
s.paths[i], s.paths[j] = s.paths[j], s.paths[i]
}

func (s sortablePaths) Less(i, j int) bool {
return s.less(s.paths, i, j)
}

func (s sortablePaths) Len() int {
return len(s.paths)
}

// FindFixedPaths returns a list of payment paths where the source and
// destination assets are fixed.
//
Expand All @@ -424,14 +442,16 @@ func (graph *OrderBookGraph) FindFixedPaths(
includePools bool,
) ([]Path, uint32, error) {
target := map[int32]bool{}

for _, destinationAsset := range destinationAssets {
destinationAssetString := destinationAsset.String()
target[graph.assetStringToID[destinationAssetString]] = true
}

sourceAssetString := sourceAsset.String()
searchState := &buyingGraphSearchState{
graph: graph,
sourceAssetString: sourceAsset.String(),
sourceAssetString: sourceAssetString,
sourceAssetAmount: amountToSpend,
targetAssets: target,
paths: []Path{},
Expand All @@ -442,7 +462,7 @@ func (graph *OrderBookGraph) FindFixedPaths(
ctx,
searchState,
maxPathLength,
graph.assetStringToID[sourceAsset.String()],
graph.assetStringToID[sourceAssetString],
amountToSpend,
)
lastLedger := graph.lastLedger
Expand All @@ -451,9 +471,11 @@ func (graph *OrderBookGraph) FindFixedPaths(
return nil, lastLedger, errors.Wrap(err, "could not determine paths")
}

sort.Slice(searchState.paths, func(i, j int) bool {
return searchState.paths[i].DestinationAmount > searchState.paths[j].DestinationAmount
})
sPaths := sortablePaths{
paths: searchState.paths,
less: compareDestinationAmount,
}
sort.Sort(sPaths)

paths, err := sortAndFilterPaths(
searchState.paths,
Expand Down Expand Up @@ -491,6 +513,10 @@ func compareDestinationAsset(allPaths []Path, i, j int) bool {
return allPaths[i].DestinationAsset < allPaths[j].DestinationAsset
}

func compareDestinationAmount(allPaths []Path, i, j int) bool {
return allPaths[i].DestinationAmount > allPaths[j].DestinationAmount
}

func sourceAssetEquals(p, otherPath Path) bool {
return p.SourceAsset == otherPath.SourceAsset
}
Expand Down Expand Up @@ -520,11 +546,13 @@ func sortAndFilterPaths(
return nil, errors.New("invalid sort by type")
}

sort.Slice(allPaths, func(i, j int) bool {
return comparePaths(allPaths, i, j)
})
sPaths := sortablePaths{
paths: allPaths,
less: comparePaths,
}
sort.Sort(sPaths)

filtered := []Path{}
filtered := make([]Path, 0, len(allPaths))
countForAsset := 0
for _, entry := range allPaths {
if len(filtered) == 0 || !assetsEqual(filtered[len(filtered)-1], entry) {
Expand Down
3 changes: 2 additions & 1 deletion exp/orderbook/graph_benchmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,8 @@ func BenchmarkVibrantPath(b *testing.B) {
b.ResetTimer()
b.ReportAllocs()
// https://horizon.stellar.org/paths/strict-send?source_asset_type=credit_alphanum4&source_asset_code=USDC&source_asset_issuer=GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVN&source_amount=10&destination_assets=ARST%3AGCSAZVWXZKWS4XS223M5F54H2B6XPIIXZZGP7KEAIU6YSL5HDRGCI3DG

// Uncomment in order to get a detailed heap allocations profile
// runtime.MemProfileRate = 1
for i := 0; i < b.N; i++ {
_, _, err := graph.FindFixedPaths(
context.Background(),
Expand Down
22 changes: 15 additions & 7 deletions exp/orderbook/search.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,9 @@ func reversePath(path []int32) {
}

func (e *pathNode) path() []int32 {
var result []int32
// Initialize slice capacity to minimize allocations.
// 8 is the maximum path supported by stellar.
result := make([]int32, 0, 8)
for cur := e; cur != nil; cur = cur.prev {
result = append(result, cur.asset)
}
Expand All @@ -128,6 +130,8 @@ func search(
bestPath := make([]*pathNode, totalAssets)
updatePath := make([]*pathNode, totalAssets)
updatedAssets := make([]int32, 0, totalAssets)
// Used to minimize allocations
slab := make([]pathNode, 0, totalAssets)
bestAmount[sourceAsset] = sourceAssetAmount
updateAmount[sourceAsset] = sourceAssetAmount
bestPath[sourceAsset] = &pathNode{
Expand Down Expand Up @@ -177,11 +181,15 @@ func search(
updateAmount[nextAsset] = nextAssetAmount

if newEntry {
updatePath[nextAsset] = &pathNode{
updatedAssets = append(updatedAssets, nextAsset)
// By piggybacking on slice appending (which uses exponential allocation)
// we avoid allocating each node individually, which is much slower and
// puts more pressure on the garbage collector.
slab = append(slab, pathNode{
asset: nextAsset,
prev: pathToCurrentAsset,
}
updatedAssets = append(updatedAssets, nextAsset)
})
updatePath[nextAsset] = &slab[len(slab)-1]
2opremio marked this conversation as resolved.
Show resolved Hide resolved
} else {
updatePath[nextAsset].prev = pathToCurrentAsset
}
Expand Down Expand Up @@ -259,9 +267,9 @@ func (state *sellingGraphSearchState) betterPathAmount(currentAmount, alternativ
}

func assetIDsToAssetStrings(graph *OrderBookGraph, path []int32) []string {
var result []string
for _, asset := range path {
result = append(result, graph.idToAssetString[asset])
result := make([]string, len(path))
for i := 0; i < len(path); i++ {
result[i] = graph.idToAssetString[path[i]]
}
return result
}
Expand Down
6 changes: 3 additions & 3 deletions xdr/asset.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ func (a Asset) String() string {
return t
}

return fmt.Sprintf("%s/%s/%s", t, c, i)
return t + "/" + c + "/" + i
}

// StringCanonical returns a display friendly form of the asset following its
Expand Down Expand Up @@ -310,10 +310,10 @@ func (a Asset) Extract(typ interface{}, code interface{}, issuer interface{}) er
switch a.Type {
case AssetTypeAssetTypeCreditAlphanum4:
an := a.MustAlphaNum4()
*code = strings.TrimRight(string(an.AssetCode[:]), "\x00")
*code = string(trimRightZeros(an.AssetCode[:]))
case AssetTypeAssetTypeCreditAlphanum12:
an := a.MustAlphaNum12()
*code = strings.TrimRight(string(an.AssetCode[:]), "\x00")
*code = string(trimRightZeros(an.AssetCode[:]))
}
default:
return errors.New("can't extract code")
Expand Down
1 change: 1 addition & 0 deletions xdr/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ func MarshalHex(v interface{}) (string, error) {
}

// EncodingBuffer reuses internal buffers between invocations to minimize allocations.
// For that reason, it is not thread-safe.
// It intentionally only allows EncodeTo method arguments, to guarantee high performance encoding.
type EncodingBuffer struct {
encoder *xdr.Encoder
Expand Down