Skip to content

[v14] Improve performance of resource parsing via predicate expressions #39975

Merged
rosstimothy merged 1 commit intobranch/v14from
tross/backport-39972/v14
Apr 1, 2024
Merged

[v14] Improve performance of resource parsing via predicate expressions #39975
rosstimothy merged 1 commit intobranch/v14from
tross/backport-39972/v14

Conversation

@rosstimothy
Copy link
Copy Markdown
Contributor

@rosstimothy rosstimothy commented Mar 28, 2024

Backport #39929 to branch/v14

changelog: Improve performance of filtering resources via predicate expressions

…9929)

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 marked this pull request as ready for review March 28, 2024 18:52
@rosstimothy rosstimothy enabled auto-merge March 28, 2024 19:45
@rosstimothy rosstimothy added this pull request to the merge queue Apr 1, 2024
@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from nklaassen April 1, 2024 17:31
Merged via the queue into branch/v14 with commit 322cc5f Apr 1, 2024
@rosstimothy rosstimothy deleted the tross/backport-39972/v14 branch April 1, 2024 17:49
@fheinecke fheinecke mentioned this pull request Apr 12, 2024
This was referenced Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants