Skip to content

Commit

Permalink
Finding tags with filter now reduces allocations when no match
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
wblakecaldwell committed Sep 2, 2021
1 parent b60eacf commit 94f2b4b
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 46 deletions.
52 changes: 19 additions & 33 deletions template/tree_v4.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion template/tree_v4_manual.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}
22 changes: 11 additions & 11 deletions template/tree_v4_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand All @@ -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]
}

Expand Down
2 changes: 1 addition & 1 deletion template/tree_v6_manual.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}

0 comments on commit 94f2b4b

Please sign in to comment.