perf: Improve resmap performance with AppendAll and Transform functions
Personal notes, keeping as draft :)
Rebased, since the SortOrderTransformer PR is in
With the benchmark suite, this PR reduces the runtime from 48s to 2.5s on my machine. I think we could split it into multiple independent PRs now. I've benchmarked them on their own to see where the major effect
perf: ResAccumulator: Use map for resmap intersection
standalone. 48s to 47s for the benchmark.
Transform
perf: NamespaceTransformer: Use Transform function for conflict detection
perf: resmap: Add Transform function
pair of PRs, 48s to 47s for the benchmark
AppendMany
commit f5cdc84e6b490a5235f4002efdd5d884310def39 perf: resmap: Add AppendMany function no effect on its own
commit e06a96957bb474bbff22ae4940934ff1e8692a47 perf: improve applyOrdering by avoid call to GetByCurrentId little effect on its own
commit f6ed5d59f998a8b5f75833faf1fe2e66358f54af perf: Improve SortOrderTransformer and use AppendMany commit 0fe542166a28b7a404a5f4241862d228df6bcb87 perf: Use AppendMany for SortOrderTransformer/newResMapFromResourceSlice/appendAll should split this. Here's the major change 48s->2.5s Could split this into more pieces.
This PR has multiple commits, and the default merge method is: merge. You can request commits to be squashed using the label: tide/merge-method-squash
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.
Hi @chlunde. Thanks for your PR.
I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test label.
I understand the commands that are listed here.
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.
@annasong20 would you like this as one PR, or do you prefer to split some of the commits into different three PRs? I've attempted to make it easier to remove by having a logical story if you start at the first commit, except for perf: improve applyOrdering by avoid call to GetByCurrentId which is actually another PR that this depends on.
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.
This bot triages PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the PR is closed
You can:
- Mark this PR as fresh with
/remove-lifecycle stale - Close this PR with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
/remove-lifecycle stale
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: chlunde Once this PR has been reviewed and has the lgtm label, please assign knverey for approval. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.
This bot triages PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the PR is closed
You can:
- Mark this PR as fresh with
/remove-lifecycle stale - Close this PR with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
/remove-lifecycle stale /auto-cc
On Mon, Jul 15, 2024, 4:23 p.m. Kubernetes Triage Robot < @.***> wrote:
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.
This bot triages PRs according to the following rules:
- After 90d of inactivity, lifecycle/stale is applied
- After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
- After 30d of inactivity since lifecycle/rotten was applied, the PR is closed
You can:
- Mark this PR as fresh with /remove-lifecycle stale
- Close this PR with /close
- Offer to help out with Issue Triage https://www.kubernetes.dev/docs/guide/issue-triage/
Please send feedback to sig-contributor-experience at kubernetes/community https://github.com/kubernetes/community.
/lifecycle stale
— Reply to this email directly, view it on GitHub https://github.com/kubernetes-sigs/kustomize/pull/5427#issuecomment-2229335390, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFKQYME3E5I422CCVKRGXDZMQVVZAVCNFSM6AAAAAA6WWTZC2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMRZGMZTKMZZGA . You are receiving this because you are subscribed to this thread.Message ID: @.***>
@stormqueen1990 Could I assign you for this and https://github.com/kubernetes-sigs/kustomize/pull/5425 PR?
@koba1t yes, absolutely!
Hi there, @chlunde! 👋🏻
I noticed this PR is still in draft state. Would you like to have it be reviewed again?
I tried to run CI on GitHub Actions. /assign @stormqueen1990