From 94f2b4b98276939729c217ed336a8bd22adec731 Mon Sep 17 00:00:00 2001 From: Blake Caldwell Date: Thu, 2 Sep 2021 03:36:46 +0000 Subject: [PATCH] Finding tags with filter now reduces allocations when no match The methods that take a filter no longer allocate first, then filter out. We now only add them to the slice if they match. Did this by putting the main logic in the *Filter methods, then having the non-Filter methods call them, with a nil filter. --- template/tree_v4.go | 52 ++++++++++++++------------------------ template/tree_v4_manual.go | 2 +- template/tree_v4_test.go | 22 ++++++++-------- template/tree_v6_manual.go | 2 +- 4 files changed, 32 insertions(+), 46 deletions(-) diff --git a/template/tree_v4.go b/template/tree_v4.go index 6d2ed65..f320cd1 100644 --- a/template/tree_v4.go +++ b/template/tree_v4.go @@ -79,10 +79,9 @@ func (t *TreeV4) addTag(tag GeneratedType, nodeIndex uint, matchFunc MatchesFunc return ret } -// return the tags at the input node index - appending to the input slice -// - nil if none found +// return the tags at the input node index - appending to the input slice if they pass the optional filter func // - ret is only appended to -func (t *TreeV4) tagsForNode(ret []GeneratedType, nodeIndex uint) []GeneratedType { +func (t *TreeV4) tagsForNode(ret []GeneratedType, nodeIndex uint, filterFunc FilterFunc) []GeneratedType { if nodeIndex == 0 { // useful for base cases where we haven't found anything return ret @@ -92,7 +91,10 @@ func (t *TreeV4) tagsForNode(ret []GeneratedType, nodeIndex uint) []GeneratedTyp tagCount := t.nodes[nodeIndex].TagCount key := uint64(nodeIndex) << 32 for i := 0; i < tagCount; i++ { - ret = append(ret, t.tags[key+uint64(i)]) + tag := t.tags[key+uint64(i)] + if filterFunc == nil || filterFunc(tag) { + ret = append(ret, tag) + } } return ret } @@ -118,7 +120,7 @@ func (t *TreeV4) firstTagForNode(nodeIndex uint) GeneratedType { func (t *TreeV4) deleteTag(buf []GeneratedType, nodeIndex uint, matchTag GeneratedType, matchFunc MatchesFunc) (int, int) { // get tags buf = buf[:0] - buf = t.tagsForNode(buf, nodeIndex) + buf = t.tagsForNode(buf, nodeIndex, nil) if len(buf) == 0 { return 0, 0 } @@ -476,26 +478,9 @@ func (t *TreeV4) FindTagsWithFilter(address patricia.IPv4Address, filterFunc Fil return t.FindTagsWithFilterAppend(ret, address, filterFunc) } -// FindTagsWithFilterAppend finds all matching tags that passes the filter function -// - results are appended to the input slice -func (t *TreeV4) FindTagsWithFilterAppend(ret []GeneratedType, address patricia.IPv4Address, filterFunc FilterFunc) []GeneratedType { - retPos := len(ret) - ret = t.FindTagsAppend(ret, address) - - if len(ret) == retPos || filterFunc == nil { - return ret - } - - // filter in place - length := len(ret) - for i := retPos; i < length; i++ { - val := ret[i] - if filterFunc(val) { - ret[retPos] = val - retPos++ - } - } - return ret[:retPos] +// FindTagsAppend finds all matching tags for given address and appends them to ret +func (t *TreeV4) FindTagsAppend(ret []GeneratedType, address patricia.IPv4Address) []GeneratedType { + return t.FindTagsWithFilterAppend(ret, address, nil) } // FindTags finds all matching tags for given address @@ -505,13 +490,14 @@ func (t *TreeV4) FindTags(address patricia.IPv4Address) []GeneratedType { return t.FindTagsAppend(ret, address) } -// FindTagsAppend finds all matching tags for given address and appends them to ret -func (t *TreeV4) FindTagsAppend(ret []GeneratedType, address patricia.IPv4Address) []GeneratedType { +// FindTagsWithFilterAppend finds all matching tags that passes the filter function +// - results are appended to the input slice +func (t *TreeV4) FindTagsWithFilterAppend(ret []GeneratedType, address patricia.IPv4Address, filterFunc FilterFunc) []GeneratedType { var matchCount uint root := &t.nodes[1] if root.TagCount > 0 { - ret = t.tagsForNode(ret, 1) + ret = t.tagsForNode(ret, 1, filterFunc) } if address.Length == 0 { @@ -541,7 +527,7 @@ func (t *TreeV4) FindTagsAppend(ret []GeneratedType, address patricia.IPv4Addres // matched the full node - get its tags, then chop off the bits we've already matched and continue if node.TagCount > 0 { - ret = t.tagsForNode(ret, nodeIndex) + ret = t.tagsForNode(ret, nodeIndex, filterFunc) } if matchCount == address.Length { @@ -638,7 +624,7 @@ func (t *TreeV4) FindDeepestTagsAppend(ret []GeneratedType, address patricia.IPv if address.Length == 0 { // caller just looking for root tags - return found, t.tagsForNode(ret, retTagIndex) + return found, t.tagsForNode(ret, retTagIndex, nil) } var nodeIndex uint @@ -651,14 +637,14 @@ func (t *TreeV4) FindDeepestTagsAppend(ret []GeneratedType, address patricia.IPv // traverse the tree for { if nodeIndex == 0 { - return found, t.tagsForNode(ret, retTagIndex) + return found, t.tagsForNode(ret, retTagIndex, nil) } node := &t.nodes[nodeIndex] matchCount := node.MatchCount(address) if matchCount < node.prefixLength { // didn't match the entire node - we're done - return found, t.tagsForNode(ret, retTagIndex) + return found, t.tagsForNode(ret, retTagIndex, nil) } // matched the full node - get its tags, then chop off the bits we've already matched and continue @@ -669,7 +655,7 @@ func (t *TreeV4) FindDeepestTagsAppend(ret []GeneratedType, address patricia.IPv if matchCount == address.Length { // exact match - we're done - return found, t.tagsForNode(ret, retTagIndex) + return found, t.tagsForNode(ret, retTagIndex, nil) } // there's still more address - keep traversing diff --git a/template/tree_v4_manual.go b/template/tree_v4_manual.go index 0d18b52..106cbd6 100644 --- a/template/tree_v4_manual.go +++ b/template/tree_v4_manual.go @@ -26,6 +26,6 @@ func (t *TreeV4) print() { buf := make([]GeneratedType, 0) for i := range t.nodes { buf = buf[:0] - fmt.Printf("%d: \tleft: %d, right: %d, prefix: %032b (%d), tags: (%d): %v\n", i, int(t.nodes[i].Left), int(t.nodes[i].Right), int(t.nodes[i].prefix), int(t.nodes[i].prefixLength), t.nodes[i].TagCount, t.tagsForNode(buf, uint(i))) + fmt.Printf("%d: \tleft: %d, right: %d, prefix: %032b (%d), tags: (%d): %v\n", i, int(t.nodes[i].Left), int(t.nodes[i].Right), int(t.nodes[i].prefix), int(t.nodes[i].prefixLength), t.nodes[i].TagCount, t.tagsForNode(buf, uint(i), nil)) } } diff --git a/template/tree_v4_test.go b/template/tree_v4_test.go index ccd7bfb..46e77a6 100644 --- a/template/tree_v4_test.go +++ b/template/tree_v4_test.go @@ -1031,13 +1031,13 @@ func TestDelete1(t *testing.T) { // verify status of internal nodes collections assert.Zero(t, len(tree.availableIndexes)) - assert.Equal(t, "tagZ", tree.tagsForNode(tags, 1)[0]) + assert.Equal(t, "tagZ", tree.tagsForNode(tags, 1, nil)[0], nil) tags = tags[:0] - assert.Equal(t, "tagA", tree.tagsForNode(tags, 2)[0]) + assert.Equal(t, "tagA", tree.tagsForNode(tags, 2, nil)[0], nil) tags = tags[:0] - assert.Equal(t, "tagB", tree.tagsForNode(tags, 3)[0]) + assert.Equal(t, "tagB", tree.tagsForNode(tags, 3, nil)[0], nil) tags = tags[:0] - assert.Equal(t, "tagC", tree.tagsForNode(tags, 4)[0]) + assert.Equal(t, "tagC", tree.tagsForNode(tags, 4, nil)[0], nil) tags = tags[:0] // three tags in a hierarchy - ask for an exact match, receive all 3 and the root @@ -1112,7 +1112,7 @@ func TestDelete1(t *testing.T) { assert.Equal(t, uint(2), tree.availableIndexes[0]) tags = tags[:0] - assert.Equal(t, "tagE", tree.tagsForNode(tags, 3)[0]) + assert.Equal(t, "tagE", tree.tagsForNode(tags, 3, nil)[0]) // 6. delete tag C count = tree.DeleteWithBuffer(tags, ipv4FromBytes([]byte{128, 3, 6, 240}, 32), matchFunc, "tagC") @@ -1153,16 +1153,16 @@ func TestTagsMap(t *testing.T) { // verify assert.Equal(t, 3, tree.nodes[1].TagCount) assert.Equal(t, "tagA", tree.firstTagForNode(1)) - assert.Equal(t, 3, len(tree.tagsForNode(tags, 1))) + assert.Equal(t, 3, len(tree.tagsForNode(tags, 1, nil))) tags = tags[:0] - assert.Equal(t, "tagA", tree.tagsForNode(tags, 1)[0]) + assert.Equal(t, "tagA", tree.tagsForNode(tags, 1, nil)[0]) tags = tags[:0] - assert.Equal(t, "tagB", tree.tagsForNode(tags, 1)[1]) + assert.Equal(t, "tagB", tree.tagsForNode(tags, 1, nil)[1]) tags = tags[:0] - assert.Equal(t, "tagC", tree.tagsForNode(tags, 1)[2]) + assert.Equal(t, "tagC", tree.tagsForNode(tags, 1, nil)[2]) tags = tags[:0] // delete tagB @@ -1176,10 +1176,10 @@ func TestTagsMap(t *testing.T) { assert.Equal(t, 1, deleted) assert.Equal(t, 2, kept) assert.Equal(t, 2, tree.nodes[1].TagCount) - assert.Equal(t, "tagA", tree.tagsForNode(tags, 1)[0]) + assert.Equal(t, "tagA", tree.tagsForNode(tags, 1, nil)[0]) tags = tags[:0] - assert.Equal(t, "tagC", tree.tagsForNode(tags, 1)[1]) + assert.Equal(t, "tagC", tree.tagsForNode(tags, 1, nil)[1]) tags = tags[:0] } diff --git a/template/tree_v6_manual.go b/template/tree_v6_manual.go index 6f36820..26688b3 100644 --- a/template/tree_v6_manual.go +++ b/template/tree_v6_manual.go @@ -26,6 +26,6 @@ func (t *TreeV6) print() { buf := make([]GeneratedType, 0) for i := range t.nodes { buf = buf[:0] - fmt.Printf("%d: \tleft: %d, right: %d, prefix: %032b %032b (%d), tags: (%d): %v\n", i, int(t.nodes[i].Left), int(t.nodes[i].Right), int(t.nodes[i].prefixLeft), int(t.nodes[i].prefixRight), int(t.nodes[i].prefixLength), t.nodes[i].TagCount, t.tagsForNode(buf, uint(i))) + fmt.Printf("%d: \tleft: %d, right: %d, prefix: %032b %032b (%d), tags: (%d): %v\n", i, int(t.nodes[i].Left), int(t.nodes[i].Right), int(t.nodes[i].prefixLeft), int(t.nodes[i].prefixRight), int(t.nodes[i].prefixLength), t.nodes[i].TagCount, t.tagsForNode(buf, uint(i), nil)) } }