Skip to content

Commit

Permalink
Simplify and reduce memory usage of MatchResourceByFilters
Browse files Browse the repository at this point in the history
```bash

$ benchstat 215637f.txt new.txt
goos: darwin
goarch: arm64
pkg: github.com/gravitational/teleport/lib/auth
                                        │ 215637f.txt │               new.txt                │
                                        │                    sec/op                    │    sec/op     vs base                │
ListUnifiedResources/labels-12                                             279.1m ± 6%   210.3m ±  2%  -24.66% (p=0.000 n=10)
ListUnifiedResources/predicate_path-12                                     509.2m ± 1%   448.5m ± 12%  -11.91% (p=0.009 n=10)
ListUnifiedResources/predicate_index-12                                    494.5m ± 4%   478.1m ±  2%        ~ (p=0.089 n=10)
ListUnifiedResources/search_lower-12                                       320.4m ± 7%   271.6m ±  4%  -15.24% (p=0.000 n=10)
ListUnifiedResources/search_upper-12                                       330.9m ± 4%   272.8m ±  4%  -17.57% (p=0.000 n=10)
geomean                                                                    375.3m        319.7m        -14.83%

                                        │ 215637f.txt │               new.txt                │
                                        │                     B/op                     │     B/op      vs base                │
ListUnifiedResources/labels-12                                            85.50Mi ± 0%   74.03Mi ± 0%  -13.42% (p=0.000 n=10)
ListUnifiedResources/predicate_path-12                                    323.7Mi ± 0%   312.2Mi ± 0%   -3.56% (p=0.000 n=10)
ListUnifiedResources/predicate_index-12                                   323.6Mi ± 0%   312.2Mi ± 0%   -3.53% (p=0.000 n=10)
ListUnifiedResources/search_lower-12                                      85.33Mi ± 0%   73.88Mi ± 0%  -13.41% (p=0.000 n=10)
ListUnifiedResources/search_upper-12                                      86.31Mi ± 0%   74.85Mi ± 0%  -13.27% (p=0.000 n=10)
geomean                                                                   145.8Mi        131.9Mi        -9.57%

                                        │ 215637f.txt │               new.txt               │
                                        │                  allocs/op                   │  allocs/op   vs base                │
ListUnifiedResources/labels-12                                             1.662M ± 0%   1.212M ± 0%  -27.09% (p=0.000 n=10)
ListUnifiedResources/predicate_path-12                                     6.613M ± 0%   6.162M ± 0%   -6.81% (p=0.000 n=10)
ListUnifiedResources/predicate_index-12                                    6.462M ± 0%   6.012M ± 0%   -6.96% (p=0.000 n=10)
ListUnifiedResources/search_lower-12                                       2.412M ± 0%   1.962M ± 0%  -18.66% (p=0.000 n=10)
ListUnifiedResources/search_upper-12                                       2.562M ± 0%   2.112M ± 0%  -17.57% (p=0.000 n=10)
geomean                                                                    3.376M        2.844M       -15.77%
```
  • Loading branch information
rosstimothy committed Mar 20, 2024
1 parent 215637f commit 6ef1702
Showing 1 changed file with 11 additions and 15 deletions.
26 changes: 11 additions & 15 deletions lib/services/matchers.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
package services

import (
"fmt"
"slices"

"github.com/gravitational/trace"
Expand Down Expand Up @@ -129,7 +128,7 @@ func MatchResourceLabels(matchers []ResourceMatcher, labels map[string]string) b
// ResourceSeenKey is used as a key for a map that keeps track
// of unique resource names and address. Currently "addr"
// only applies to resource Application.
type ResourceSeenKey struct{ name, addr string }
type ResourceSeenKey struct{ name, kind, addr string }

// MatchResourceByFilters returns true if all filter values given matched against the resource.
//
Expand All @@ -144,20 +143,21 @@ type ResourceSeenKey struct{ name, addr string }
// is not provided but is provided for kind `KubernetesCluster`.
func MatchResourceByFilters(resource types.ResourceWithLabels, filter MatchResourceFilter, seenMap map[ResourceSeenKey]struct{}) (bool, error) {
var specResource types.ResourceWithLabels
resourceKind := resource.GetKind()
kind := resource.GetKind()

// We assume when filtering for services like KubeService, AppServer, and DatabaseServer
// the user is wanting to filter the contained resource ie. KubeClusters, Application, and Database.
resourceKey := ResourceSeenKey{}
switch resourceKind {
key := ResourceSeenKey{
kind: kind,
name: resource.GetName(),
}
switch kind {
case types.KindNode,
types.KindDatabaseService,
types.KindKubernetesCluster,
types.KindWindowsDesktop, types.KindWindowsDesktopService,
types.KindUserGroup:
specResource = resource
resourceKey.name = fmt.Sprintf("%s/%s", specResource.GetName(), resourceKind)

case types.KindKubeServer:
if seenMap != nil {
return false, trace.BadParameter("checking for duplicate matches for resource kind %q is not supported", filter.ResourceKind)
Expand All @@ -170,17 +170,14 @@ func MatchResourceByFilters(resource types.ResourceWithLabels, filter MatchResou
return false, trace.BadParameter("expected types.DatabaseServer, got %T", resource)
}
specResource = server.GetDatabase()
resourceKey.name = fmt.Sprintf("%s/%s/", specResource.GetName(), resourceKind)
case types.KindAppServer, types.KindSAMLIdPServiceProvider, types.KindAppOrSAMLIdPServiceProvider:
switch appOrSP := resource.(type) {
case types.AppServer:
app := appOrSP.GetApp()
specResource = app
resourceKey.name = fmt.Sprintf("%s/%s/", specResource.GetName(), resourceKind)
resourceKey.addr = app.GetPublicAddr()
key.addr = app.GetPublicAddr()
case types.SAMLIdPServiceProvider:
specResource = appOrSP
resourceKey.name = fmt.Sprintf("%s/%s/", specResource.GetName(), resourceKind)
default:
return false, trace.BadParameter("expected types.SAMLIdPServiceProvider or types.AppServer, got %T", resource)
}
Expand All @@ -189,10 +186,9 @@ func MatchResourceByFilters(resource types.ResourceWithLabels, filter MatchResou
// of cases we need to handle. If the resource type didn't match any arm before
// and it is not a Kubernetes resource kind, we return an error.
if !slices.Contains(types.KubernetesResourcesKinds, filter.ResourceKind) {
return false, trace.NotImplemented("filtering for resource kind %q not supported", resourceKind)
return false, trace.NotImplemented("filtering for resource kind %q not supported", kind)
}
specResource = resource
resourceKey.name = fmt.Sprintf("%s/%s/", specResource.GetName(), resourceKind)
}

var match bool
Expand All @@ -211,10 +207,10 @@ func MatchResourceByFilters(resource types.ResourceWithLabels, filter MatchResou

// Deduplicate matches.
if match && seenMap != nil {
if _, exists := seenMap[resourceKey]; exists {
if _, exists := seenMap[key]; exists {
return false, nil
}
seenMap[resourceKey] = struct{}{}
seenMap[key] = struct{}{}
}

return match, nil
Expand Down

0 comments on commit 6ef1702

Please sign in to comment.