Skip to content

Commit

Permalink
Do not store non-pointer values in sync.Pool. (#4089)
Browse files Browse the repository at this point in the history
A sync.Pool is used to avoid unnecessary allocations and reduce the
amount of work the garbage collector has to do.  When passing a value
that is not a pointer to a function that accepts an interface, the value
needs to be placed on the heap, which means an additional allocation.
Slices are a common thing to put in sync.Pools, and they’re structs with
3 fields (length, capacity, and a pointer to an array). In order to
avoid the extra allocation, one should store a pointer to the slice
instead.
  • Loading branch information
martinmr authored Oct 4, 2019
1 parent bd70cba commit 155a958
Showing 1 changed file with 22 additions and 18 deletions.
40 changes: 22 additions & 18 deletions query/shortest.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ type pathInfo struct {
}

type route struct {
route []pathInfo
route *[]pathInfo
totalWeight float64
}

Expand All @@ -51,7 +51,7 @@ type queueItem struct {

var pathPool = sync.Pool{
New: func() interface{} {
return []pathInfo{}
return &[]pathInfo{}
},
}

Expand Down Expand Up @@ -284,7 +284,7 @@ func runKShortestPaths(ctx context.Context, sg *SubGraph) ([]*SubGraph, error) {
uid: sg.Params.From,
cost: 0,
hop: 0,
path: route{route: []pathInfo{{uid: sg.Params.From}}},
path: route{route: &[]pathInfo{{uid: sg.Params.From}}},
}
heap.Push(&pq, srcNode)

Expand Down Expand Up @@ -357,17 +357,21 @@ func runKShortestPaths(ctx context.Context, sg *SubGraph) ([]*SubGraph, error) {
continue
}

curPath := pathPool.Get().([]pathInfo)
if cap(curPath) < len(item.path.route)+1 {
curPath := pathPool.Get().(*[]pathInfo)
if curPath == nil {
return nil, errors.Errorf("Sync pool returned a nil pointer")
}
if cap(*curPath) < len(*item.path.route)+1 {
// We can't use it due to insufficient capacity. Put it back.
pathPool.Put(curPath)
curPath = make([]pathInfo, len(item.path.route)+1)
newSlice := make([]pathInfo, len(*item.path.route)+1)
curPath = &newSlice
} else {
// Use the curPath from pathPool. Set length appropriately.
curPath = curPath[:len(item.path.route)+1]
*curPath = (*curPath)[:len(*item.path.route)+1]
}
n := copy(curPath, item.path.route)
curPath[n] = pathInfo{
n := copy(*curPath, *item.path.route)
(*curPath)[n] = pathInfo{
uid: toUid,
attr: info.attr,
facet: info.facet,
Expand All @@ -391,7 +395,7 @@ func runKShortestPaths(ctx context.Context, sg *SubGraph) ([]*SubGraph, error) {
return nil, nil
}
var res []uint64
for _, it := range kroutes[0].route {
for _, it := range *kroutes[0].route {
res = append(res, it.uid)
}
sg.DestUIDs.Uids = res
Expand Down Expand Up @@ -641,26 +645,26 @@ func createkroutesubgraph(ctx context.Context, kroutes []route) []*SubGraph {
shortestSg.pathMeta = &pathMetadata{
weight: it.totalWeight,
}
curUid := it.route[0].uid
curUid := (*it.route)[0].uid
shortestSg.SrcUIDs = &pb.List{Uids: []uint64{curUid}}
shortestSg.DestUIDs = &pb.List{Uids: []uint64{curUid}}
shortestSg.uidMatrix = []*pb.List{{Uids: []uint64{curUid}}}

curNode := shortestSg
i := 0
for ; i < len(it.route)-1; i++ {
curUid := it.route[i].uid
childUid := it.route[i+1].uid
for ; i < len(*it.route)-1; i++ {
curUid := (*it.route)[i].uid
childUid := (*it.route)[i+1].uid
node := new(SubGraph)
node.Params = params{
Shortest: true,
}
if it.route[i+1].facet != nil {
if (*it.route)[i+1].facet != nil {
// For consistent later processing.
node.Params.Facet = &pb.FacetParams{}
}
node.Attr = it.route[i+1].attr
node.facetsMatrix = []*pb.FacetsList{{FacetsList: []*pb.Facets{it.route[i+1].facet}}}
node.Attr = (*it.route)[i+1].attr
node.facetsMatrix = []*pb.FacetsList{{FacetsList: []*pb.Facets{(*it.route)[i+1].facet}}}
node.SrcUIDs = &pb.List{Uids: []uint64{curUid}}
node.DestUIDs = &pb.List{Uids: []uint64{childUid}}
node.uidMatrix = []*pb.List{{Uids: []uint64{childUid}}}
Expand All @@ -673,7 +677,7 @@ func createkroutesubgraph(ctx context.Context, kroutes []route) []*SubGraph {
node.Params = params{
Shortest: true,
}
uid := it.route[i].uid
uid := (*it.route)[i].uid
node.SrcUIDs = &pb.List{Uids: []uint64{uid}}
node.uidMatrix = []*pb.List{{Uids: []uint64{uid}}}
curNode.Children = append(curNode.Children, node)
Expand Down

0 comments on commit 155a958

Please sign in to comment.