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

Optimize memory and execution time for RNode.GetAnnotations and RNode.GetLabels #4940

Closed
2 tasks done
ephesused opened this issue Dec 19, 2022 · 1 comment · Fixed by #4944
Closed
2 tasks done

Optimize memory and execution time for RNode.GetAnnotations and RNode.GetLabels #4940

ephesused opened this issue Dec 19, 2022 · 1 comment · Fixed by #4944
Labels
kind/feature Categorizes issue or PR as related to a new feature. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Milestone

Comments

@ephesused
Copy link
Contributor

Eschewed features

  • This issue is not requesting templating, unstuctured edits, build-time side-effects from args or env vars, or any other eschewed feature.

What would you like to have added?

I propose optimizing the handling for GetAnnotations to improve kustomize performance overall.

GetAnnotations is used heavily in complex kustomize executions, specifically in utils.PrevIds().

GetAnnotations (and GetLabels) work through VisitFields, which creates a new object (via Field) for every iterated entry. Field creates a MapNode that houses a new RNode for both a key and a value. GetAnnotations (and GetLabels) then extracts the strings from the newly created objects. The objects add no processing value for these code paths and their use is costly in both memory and execution time. Optimizing to (1) avoid new object creation and (2) restrict to only the desired annotations, there will be a significant performance improvement in both run time and memory use.

One approach may be found in PR #4810.

Why is this needed?

The key drivers are memory and performance. Eliminating the unused object creation leads to significant performance improvements. Three specific examples in PR #4810 showed run time reduced by 17%, 39%, and 82%. Memory usage was reduced by 33%, 50%, and 90%.

Can you accomplish the motivating task without this feature, and if so, how?

I believe this work is necessary in order to achieve the performance improvements.

What other solutions have you considered?

PR #4810 is one approach. I plan to submit another PR that addresses the problem in a different manner. Namely, an appropriate concern about #4810 is that it adds to the number of places that mapping node traversal is implemented. The upcoming PR will provide a common code path for mapping node traversal, and use that common code path to apply performance improvements.

Anything else we should know?

For reference, other performance gains in this area were achieved in #4790 and #4793.

Feature ownership

  • I am interested in contributing this feature myself! 🎉
@ephesused ephesused added the kind/feature Categorizes issue or PR as related to a new feature. label Dec 19, 2022
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Dec 19, 2022
@natasha41575 natasha41575 added this to the v5.0.0 milestone Dec 21, 2022
@natasha41575
Copy link
Contributor

/triage accepted

We can accept some form of performance optimization, and I think it'll be great to get it into the v5 release.

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Dec 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
3 participants