improved add filter performance

pkg/api/response.go

@@ -1,7 +1,7 @@
 package api
 
 import (
-	"fmt"
+	"strconv"
 	"time"
 )
 
@@ -163,20 +163,26 @@ type RouteServersResponse struct {
 type Community []int
 
 func (com Community) String() string {
-	res := ""
 	if len(com) < 1 {
 		return ""
 	}
-	for _, v := range com {
-		res += fmt.Sprintf(":%d", v)
+	s := ""
+	for i, v := range com {
+		if i > 0 {
+			s += ":"
+		}
+		s += strconv.Itoa(v)
 	}
-	return res[1:]
+	return s
 }
 
 // Communities is a collection of bgp communities
 type Communities []Community
 
-// Unique deduplicates communities
+// Unique deduplicates communities.
+/*
+We can skip this. Worst case is, that the
+cardinality is off.
 func (communities Communities) Unique() Communities {
 	seen := map[string]bool{}
 	result := make(Communities, 0, len(communities))
@@ -192,25 +198,34 @@ func (communities Communities) Unique() Communities {
 
 	return result
 }
+*/
 
 // ExtCommunity is a BGP extended community
 type ExtCommunity []interface{}
 
 func (com ExtCommunity) String() string {
-	res := ""
 	if len(com) < 1 {
 		return ""
 	}
-	for _, v := range com {
-		res += fmt.Sprintf(":%v", v)
+	res := ""
+	for i, v := range com {
+		if i == 0 {
+			res += v.(string)
+			continue
+		}
+		if i > 0 {
+			res += ":"
+		}
+		res += strconv.Itoa(v.(int))
 	}
-	return res[1:]
+	return res
 }
 
 // ExtCommunities is a collection of extended bgp communities.
 type ExtCommunities []ExtCommunity
 
 // Unique deduplicates extended communities.
+/*
 func (communities ExtCommunities) Unique() ExtCommunities {
 	seen := map[string]bool{}
 	result := make(ExtCommunities, 0, len(communities))
@@ -226,6 +241,7 @@ func (communities ExtCommunities) Unique() ExtCommunities {
 
 	return result
 }
+*/
 
 // BGPInfo is a set of BGP attributes
 type BGPInfo struct {

pkg/api/response_test.go

@@ -74,7 +74,7 @@ func TestCommunityStringify(t *testing.T) {
 		t.Error("Expected 23:42, got:", com.String())
 	}
 
-	extCom := ExtCommunity{"ro", "42", "123"}
+	extCom := ExtCommunity{"ro", 42, 123}
 	if extCom.String() != "ro:42:123" {
 		t.Error("Expected ro:42:123, but got:", extCom.String())
 	}
@@ -134,6 +134,7 @@ func TestHasCommunity(t *testing.T) {
 	}
 }
 
+/*
 func TestUniqueCommunities(t *testing.T) {
 	all := Communities{Community{23, 42}, Community{42, 123}, Community{23, 42}}
 	unique := all.Unique()
@@ -154,3 +155,4 @@ func TestUniqueExtCommunities(t *testing.T) {
 	}
 	t.Log("All:", all, "Unique:", unique)
 }
+*/

pkg/api/search_filters.go

@@ -1,7 +1,6 @@
 package api
 
 import (
-	"fmt"
 	"log"
 	"net/url"
 	"strconv"
@@ -131,14 +130,27 @@ func (g *SearchFilterGroup) Contains(filter *SearchFilter) bool {
 	return g.FindFilter(filter) != nil
 }
 
+// filterValueAsString gets the string representation
+// from a filter value
+func filterValueAsString(value interface{}) string {
+	switch v := value.(type) {
+	case int:
+		return strconv.Itoa(v)
+	case string:
+		return v
+	case Community:
+		return v.String()
+	case ExtCommunity:
+		return v.String()
+	}
+	panic("unexpected filter value")
+}
+
 // GetFilterByValue retrieves a filter by matching
 // a string representation of it's filter value.
 func (g *SearchFilterGroup) GetFilterByValue(value interface{}) *SearchFilter {
-	// I've tried it with .(fmt.Stringer), but int does not implement this...
-	// So whatever. I'm using the trick of letting Sprintf choose the right
-	// conversion. If this is too expensive, we need to refactor this.
-	// TODO: profile this.
-	idx, ok := g.filtersIdx[fmt.Sprintf("%v", value)]
+	ref := filterValueAsString(value)
+	idx, ok := g.filtersIdx[ref]
 	if !ok {
 		return nil // We don't have this particular filter
 	}
@@ -158,7 +170,8 @@ func (g *SearchFilterGroup) AddFilter(filter *SearchFilter) {
 	idx := len(g.Filters)
 	filter.Cardinality = 1
 	g.Filters = append(g.Filters, filter)
-	g.filtersIdx[fmt.Sprintf("%v", filter.Value)] = idx
+	ref := filterValueAsString(filter.Value)
+	g.filtersIdx[ref] = idx
 }
 
 // AddFilters adds a list of filters to a group.
@@ -172,7 +185,8 @@ func (g *SearchFilterGroup) AddFilters(filters []*SearchFilter) {
 func (g *SearchFilterGroup) rebuildIndex() {
 	idx := make(map[string]int)
 	for i, filter := range g.Filters {
-		idx[fmt.Sprintf("%v", filter.Value)] = i
+		ref := filterValueAsString(filter.Value)
+		idx[ref] = i
 	}
 	g.filtersIdx = idx // replace index
 }
@@ -368,21 +382,21 @@ func (s *SearchFilters) UpdateFromLookupRoute(r *LookupRoute) {
 
 	// Add communities
 	communities := s.GetGroupByKey(SearchKeyCommunities)
-	for _, c := range r.Route.BGP.Communities.Unique() {
+	for _, c := range r.Route.BGP.Communities {
 		communities.AddFilter(&SearchFilter{
 			Name:  c.String(),
 			Value: c,
 		})
 	}
 	extCommunities := s.GetGroupByKey(SearchKeyExtCommunities)
-	for _, c := range r.Route.BGP.ExtCommunities.Unique() {
+	for _, c := range r.Route.BGP.ExtCommunities {
 		extCommunities.AddFilter(&SearchFilter{
 			Name:  c.String(),
 			Value: c,
 		})
 	}
 	largeCommunities := s.GetGroupByKey(SearchKeyLargeCommunities)
-	for _, c := range r.Route.BGP.LargeCommunities.Unique() {
+	for _, c := range r.Route.BGP.LargeCommunities {
 		largeCommunities.AddFilter(&SearchFilter{
 			Name:  c.String(),
 			Value: c,
@@ -398,21 +412,21 @@ func (s *SearchFilters) UpdateFromRoute(r *Route) {
 
 	// Add communities
 	communities := s.GetGroupByKey(SearchKeyCommunities)
-	for _, c := range r.BGP.Communities.Unique() {
+	for _, c := range r.BGP.Communities {
 		communities.AddFilter(&SearchFilter{
 			Name:  c.String(),
 			Value: c,
 		})
 	}
 	extCommunities := s.GetGroupByKey(SearchKeyExtCommunities)
-	for _, c := range r.BGP.ExtCommunities.Unique() {
+	for _, c := range r.BGP.ExtCommunities {
 		extCommunities.AddFilter(&SearchFilter{
 			Name:  c.String(),
 			Value: c,
 		})
 	}
 	largeCommunities := s.GetGroupByKey(SearchKeyLargeCommunities)
-	for _, c := range r.BGP.LargeCommunities.Unique() {
+	for _, c := range r.BGP.LargeCommunities {
 		largeCommunities.AddFilter(&SearchFilter{
 			Name:  c.String(),
 			Value: c,