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.

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

[release-0.16] 🏃 Make client.MatchingLabels faster #6

Merged

Conversation

timflannagan
Copy link

Carries the upstream commit that fixes the client.MatchingLabels performance regression we were seeing in various product codebases. This also picks up another upstream commit that has landed in the release-0.16 branch since this branch was last created:

$ git diff upstream/release-0.16
diff --git a/pkg/client/options.go b/pkg/client/options.go
index d81bf25d..ee6489b7 100644
--- a/pkg/client/options.go
+++ b/pkg/client/options.go
@@ -514,7 +514,8 @@ type MatchingLabels map[string]string
 func (m MatchingLabels) ApplyToList(opts *ListOptions) {
        // TODO(directxman12): can we avoid reserializing this over and over?
        if opts.LabelSelector == nil {
-               opts.LabelSelector = labels.NewSelector()
+               opts.LabelSelector = labels.SelectorFromValidatedSet(map[string]string(m))
+               return
        }
        // If there's already a selector, we need to AND the two together.
        noValidSel := labels.SelectorFromValidatedSet(map[string]string(m))

Now, the only differences between the upstream and downstream release-0.16 branches are we picked up a commit that hasn't been backported.

timflannagan and others added 3 commits October 11, 2023 13:03
The 99% use-case of this is to set a selector, not to adjust an existing
one. This change introduces a fastpath that does that with half the
allocations and in a bit less than half the time.

The reason slowpath is slow is that for each label a requirement has to
be constructed that is then appended to a slice, both of which cause
allocations.
…that themselves have no explicit config

This change allows to define a cache selector config that applies to all
namespaces that themselves do not have an explicit config. An example
would be "Cache all namespaces without selector, except for namespace
foo, there use labelSelector bar=baz".

More as a side effect than intentionally, this also makes it valid to use
the empty string as a key in the `Namespaces` and `byObject.Namespaces`
config of the cache. This is very useful to for example have a
`namespace` CLI flag that might be empty.

This change is the last missing bit to finish the implementation of the
[cache options design doc](./designs/cache_options.md).
@timflannagan timflannagan merged commit eaaf19f into release-0.16 Oct 11, 2023
3 checks passed
@timflannagan timflannagan deleted the release-0.16-chore/carry-labels-perf-regression branch October 11, 2023 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants