Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[v15] Improve performance of resource filtering via predicate expressions #39972

Merged
merged 1 commit into from Mar 28, 2024
Merged

[v15] Improve performance of resource filtering via predicate expressions #39972

merged 1 commit into from Mar 28, 2024

Conversation

rosstimothy
Copy link
Contributor

Backport #39929 to branch/v15

changelog: Improve performance of filtering resources via predicate expressions

@rosstimothy
Improve performance of resource parsing via predicate expressions
b22a7f1 
The existing resource parser was converted to use typical under
the hood. Aside from the type safety improvements the making the
switch allows a single predicate expression to be parsed once and
evaluated per resource. The previous parser worked backwards, it
created a parser per resource with the same expression which lead
to elevated memory consumption and poor performance.

An additional bug was fixed that prevented predicate filtering from
working properly with the unified resource API due to applying
predicates prior to filtering by kind. This resulted in queriers
for server specific things (resource.spec.hostname) to fail even
when only the server resource was specified in the resource kinds.

Benchamrk results for the changes in this commit compared to master
show a 75% reduction in the test time and a 96% reduction in allocations
during the tests.

```bash

$ benchstat e77e5e0.txt 8eff752a93dfe96225a71431af829015e83400fa.txt
goos: darwin
goarch: arm64
pkg: github.com/gravitational/teleport/lib/auth
                                              │ e77e5e0.txt │ 8eff752a93dfe96225a71431af829015e83400fa.txt │
                                              │                    sec/op                    │        sec/op         vs base                │
ListUnifiedResourcesFilter/labels-12                                            73.88m ±  1%            64.74m ± 6%  -12.37% (p=0.000 n=10)
ListUnifiedResourcesFilter/predicate_path-12                                   294.06m ± 20%            74.83m ± 5%  -74.55% (p=0.000 n=10)
ListUnifiedResourcesFilter/predicate_index-12                                  307.63m ±  4%            76.32m ± 4%  -75.19% (p=0.000 n=10)
ListUnifiedResourcesFilter/search_lower-12                                      130.7m ±  5%            130.5m ± 4%        ~ (p=0.971 n=10)
ListUnifiedResourcesFilter/search_upper-12                                      147.4m ±  5%            138.7m ± 2%   -5.87% (p=0.003 n=10)
ListUnifiedResources/simple_labels-12                                            1.748 ±  2%             1.744 ± 2%        ~ (p=0.631 n=10)
geomean                                                                         246.6m                  150.6m       -38.93%

                                              │ e77e5e0.txt │ 8eff752a93dfe96225a71431af829015e83400fa.txt │
                                              │                     B/op                     │         B/op          vs base                │
ListUnifiedResourcesFilter/labels-12                                            735.2Ki ± 0%           724.6Ki ± 1%   -1.44% (p=0.000 n=10)
ListUnifiedResourcesFilter/predicate_path-12                                  238.873Mi ± 0%           7.585Mi ± 0%  -96.82% (p=0.000 n=10)
ListUnifiedResourcesFilter/predicate_index-12                                 238.869Mi ± 0%           7.587Mi ± 0%  -96.82% (p=0.000 n=10)
ListUnifiedResourcesFilter/search_lower-12                                      16.77Mi ± 0%           16.77Mi ± 0%        ~ (p=0.436 n=10)
ListUnifiedResourcesFilter/search_upper-12                                      16.78Mi ± 0%           16.76Mi ± 0%        ~ (p=0.052 n=10)
ListUnifiedResources/simple_labels-12                                           1.272Gi ± 0%           1.273Gi ± 0%        ~ (p=0.739 n=10)
geomean                                                                         49.67Mi                15.69Mi       -68.41%

                                              │ e77e5e0.txt │ 8eff752a93dfe96225a71431af829015e83400fa.txt │
                                              │                  allocs/op                   │      allocs/op        vs base                │
ListUnifiedResourcesFilter/labels-12                                             11.45k ± 0%            11.41k ± 0%   -0.37% (p=0.000 n=10)
ListUnifiedResourcesFilter/predicate_path-12                                    4962.0k ± 0%            161.5k ± 0%  -96.75% (p=0.000 n=10)
ListUnifiedResourcesFilter/predicate_index-12                                   4812.0k ± 0%            161.5k ± 0%  -96.64% (p=0.000 n=10)
ListUnifiedResourcesFilter/search_lower-12                                       161.6k ± 0%            161.6k ± 0%        ~ (p=0.419 n=10)
ListUnifiedResourcesFilter/search_upper-12                                       161.7k ± 0%            161.6k ± 0%        ~ (p=0.116 n=10)
ListUnifiedResources/simple_labels-12                                            23.70M ± 0%            23.70M ± 0%   +0.01% (p=0.019 n=10)
geomean                                                                          743.7k                 238.5k       -67.93%
```
@rosstimothy rosstimothy added this pull request to the merge queue Mar 28, 2024
Merged via the queue into branch/v15 with commit 8eda5e5 Mar 28, 2024
34 checks passed
@rosstimothy rosstimothy deleted the bot/backport-39929-branch/v15 branch March 28, 2024 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@espadolini espadolini espadolini approved these changes

@zmb3 zmb3 zmb3 approved these changes

Assignees
No one assigned
Labels
backport kubernetes-access size/sm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@rosstimothy @espadolini @zmb3