From bf95c539ce7cd0f81316644b28820398a57fcefb Mon Sep 17 00:00:00 2001 From: Annika Hannig Date: Fri, 25 Nov 2022 15:38:38 +0100 Subject: [PATCH] improved add filter performance --- pkg/api/response.go | 36 +++++++++++++++++++++++---------- pkg/api/response_test.go | 4 +++- pkg/api/search_filters.go | 42 ++++++++++++++++++++++++++------------- 3 files changed, 57 insertions(+), 25 deletions(-) diff --git a/pkg/api/response.go b/pkg/api/response.go index 9754bf4..642a44e 100644 --- a/pkg/api/response.go +++ b/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 { diff --git a/pkg/api/response_test.go b/pkg/api/response_test.go index 6941a50..f32c415 100644 --- a/pkg/api/response_test.go +++ b/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) } +*/ diff --git a/pkg/api/search_filters.go b/pkg/api/search_filters.go index ac5a5a2..d8e17be 100644 --- a/pkg/api/search_filters.go +++ b/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,