Extract loop constant from makeResIds.go's PrevIds for performance improvement

[ephesused]

Describe the bug

https://github.com/kubernetes-sigs/kustomize/blob/0fa010c/api/internal/utils/makeResIds.go#L53-L57 makes somewhat heavy calls, but it is a loop constant. Moving the constant out of the loop can yield a significant performance improvement.

Apologies for the lack of a PR. I haven't gotten myself set up properly yet, and figured I'd send this along in an issue since it's a simple adjustment.

makeResIds.patch.txt

Comparison

[kustomize]# git status
HEAD detached at kustomize/v4.5.7
Untracked files:
  (use "git add <file>..." to include in what will be committed)
        makeResIds.patch

nothing added to commit but untracked files present (use "git add" to track)
[kustomize]# (cd kustomize; go build)
[kustomize]# time kustomize/kustomize -o /tmp/v4.5.7.yaml build /tmp/generate/order-data-science-simple

real    0m37.577s
user    0m52.042s
sys     0m0.868s
[kustomize]# git apply makeResIds.patch
[kustomize]# (cd kustomize; go build)
[kustomize]# time kustomize/kustomize -o /tmp/patched.yaml build /tmp/generate/order-data-science-simple

real    0m17.872s
user    0m23.712s
sys     0m0.565s
[kustomize]# diff /tmp/v4.5.7.yaml /tmp/patched.yaml
[kustomize]# 

Kustomize version

v4.5.7

Platform

linux

[natasha41575]

/triage accepted
/help

[k8s-ci-robot]

@natasha41575:
This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

• Why are we solving this issue?
• To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
• Does this issue have zero to low barrier of entry?
• How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

/triage accepted
/help

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[caproven]

I can try creating a PR for this. Want to run my own performance testing but thanks @ephesused for finding this!

[natasha41575]

@caproven Feel free to assign this issue to yourself (you can leave a comment with /assign to do so).

[caproven]

Thanks @natasha41575

/assign

[caproven]

I can confirm that @ephesused was on to something, excellent find! Below are some performance findings of mine based on a large set of manifests whose total output is ~70k lines with heavy patchTransformer usage, run on a Ryzen 5 5600x.

Before:

• 257.82s user 17.40s system 200% cpu 2:17.22 total
• 19,952,711 invocations of GetMeta() & ParseGroupVersion() from PrevIds()

After:

• 89.04s user 3.04s system 196% cpu 46.883 total
• 2,923,267 invocations of GetMeta() & ParseGroupVersion() from PrevIds()

[ephesused]

I think I see a way to improve further. Starting with the above fix, I have a complex run that executes in about 300s. I can get that down to about 220s by adjusting the PrevIds's GetMeta call so that it tells GetMeta to populate only what PrevIds needs: the APIVersion.

The hack I'm testing allows callers to restrict the set of fields that GetMeta populates. Currently GetMeta always populates everything. With the number of calls we're seeing for complex runs, that full population for PrevIds is fairly expensive (and carries no benefit).

Perhaps this sort of adjustment is in line with the intent of the "// TODO: consider optimizing this parsing" comment in GetMeta.

I'll open a separate issue once I've got this baked a little more. I'll use that issue as my primer to be a contributor.

Edited for grammar/typos