From 5c7f8b8d73679cf5f94cb5dcc5c19d26130253a0 Mon Sep 17 00:00:00 2001 From: Carl Henrik Lunde Date: Tue, 7 Mar 2023 21:12:53 +0100 Subject: [PATCH] perf: improve applyOrdering by avoid call to GetByCurrentId This shaves of 14 seconds (one third) of the execution time for a kustomization tree with 4000 documents, from 40.68s to 27.41s 0 0% 5.44% 18.42s 40.56% sigs.k8s.io/kustomize/api/krusty.(*Kustomizer).applySortOrder 0 0% 5.44% 18.40s 40.52% sigs.k8s.io/kustomize/api/internal/builtins.applyOrdering before (pprof) top20 -cum Showing nodes accounting for 5.85s, 12.88% of 45.41s total Dropped 622 nodes (cum <= 0.23s) Showing top 20 nodes out of 157 flat flat% sum% cum cum% 0 0% 0% 40.68s 89.58% github.com/spf13/cobra.(*Command).Execute 0 0% 0% 40.68s 89.58% github.com/spf13/cobra.(*Command).ExecuteC 0 0% 0% 40.68s 89.58% github.com/spf13/cobra.(*Command).execute 0 0% 0% 40.68s 89.58% main.main 0 0% 0% 40.68s 89.58% runtime.main 0 0% 0% 40.68s 89.58% sigs.k8s.io/kustomize/kustomize/v5/commands/build.NewCmdBuild.func1 0 0% 0% 40.12s 88.35% sigs.k8s.io/kustomize/api/krusty.(*Kustomizer).Run 0.51s 1.12% 1.12% 33.20s 73.11% sigs.k8s.io/kustomize/api/resource.(*Resource).CurId 0 0% 1.12% 26.95s 59.35% sigs.k8s.io/kustomize/api/resmap.(*resWrangler).GetMatchingResourcesByCurrentId 0.35s 0.77% 1.89% 26.95s 59.35% sigs.k8s.io/kustomize/api/resmap.(*resWrangler).filteredById 0.07s 0.15% 2.05% 25.53s 56.22% sigs.k8s.io/kustomize/api/resmap.GetCurrentId 0 0% 2.05% 21.68s 47.74% sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).MakeCustomizedResMap (inline) 0 0% 2.05% 21.68s 47.74% sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).makeCustomizedResMap 0.54s 1.19% 3.24% 19.75s 43.49% sigs.k8s.io/kustomize/api/resource.(*Resource).GetGvk (inline) 1s 2.20% 5.44% 19.21s 42.30% sigs.k8s.io/kustomize/kyaml/resid.GvkFromNode 0 0% 5.44% 18.42s 40.56% sigs.k8s.io/kustomize/api/internal/builtins.(*SortOrderTransformerPlugin).Transform 0 0% 5.44% 18.42s 40.56% sigs.k8s.io/kustomize/api/krusty.(*Kustomizer).applySortOrder 0 0% 5.44% 18.40s 40.52% sigs.k8s.io/kustomize/api/internal/builtins.applyOrdering 0.87s 1.92% 7.36% 16.55s 36.45% sigs.k8s.io/kustomize/kyaml/yaml.visitMappingNodeFields 2.51s 5.53% 12.88% 15.68s 34.53% sigs.k8s.io/kustomize/kyaml/yaml.visitFieldsWhileTrue after (pprof) top20 -cum Showing nodes accounting for 1.23s, 3.85% of 31.98s total Dropped 584 nodes (cum <= 0.16s) Showing top 20 nodes out of 184 flat flat% sum% cum cum% 0 0% 0% 27.41s 85.71% github.com/spf13/cobra.(*Command).Execute 0 0% 0% 27.41s 85.71% github.com/spf13/cobra.(*Command).ExecuteC 0 0% 0% 27.41s 85.71% github.com/spf13/cobra.(*Command).execute 0 0% 0% 27.41s 85.71% main.main 0 0% 0% 27.41s 85.71% runtime.main 0 0% 0% 27.41s 85.71% sigs.k8s.io/kustomize/kustomize/v5/commands/build.NewCmdBuild.func1 0 0% 0% 26.85s 83.96% sigs.k8s.io/kustomize/api/krusty.(*Kustomizer).Run 0 0% 0% 22.07s 69.01% sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).MakeCustomizedResMap (inline) 0 0% 0% 22.07s 69.01% sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).makeCustomizedResMap 0.38s 1.19% 1.19% 20.69s 64.70% sigs.k8s.io/kustomize/api/resource.(*Resource).CurId 0 0% 1.19% 13.64s 42.65% sigs.k8s.io/kustomize/api/resmap.(*resWrangler).Append 0 0% 1.19% 13.55s 42.37% sigs.k8s.io/kustomize/api/resmap.(*resWrangler).GetMatchingResourcesByCurrentId (inline) 0.12s 0.38% 1.56% 13.55s 42.37% sigs.k8s.io/kustomize/api/resmap.(*resWrangler).filteredById 0.01s 0.031% 1.59% 12.67s 39.62% sigs.k8s.io/kustomize/api/resmap.GetCurrentId 0.21s 0.66% 2.25% 12.49s 39.06% sigs.k8s.io/kustomize/api/resource.(*Resource).GetGvk (inline) 0.51s 1.59% 3.85% 12.28s 38.40% sigs.k8s.io/kustomize/kyaml/resid.GvkFromNode 0 0% 3.85% 11.52s 36.02% sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).IgnoreLocal 0 0% 3.85% 10.53s 32.93% sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).AccumulateTarget 0 0% 3.85% 10.53s 32.93% sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).accumulateResources 0 0% 3.85% 10.53s 32.93% sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).accumulateTarget --- api/internal/builtins/SortOrderTransformer.go | 53 ++++++++----------- .../SortOrderTransformer.go | 53 ++++++++----------- 2 files changed, 44 insertions(+), 62 deletions(-) diff --git a/api/internal/builtins/SortOrderTransformer.go b/api/internal/builtins/SortOrderTransformer.go index 687f21a608..c50f627fee 100644 --- a/api/internal/builtins/SortOrderTransformer.go +++ b/api/internal/builtins/SortOrderTransformer.go @@ -74,34 +74,16 @@ func (p *SortOrderTransformerPlugin) Transform(m resmap.ResMap) (err error) { // Sort if p.SortOptions.Order == types.LegacySortOrder { - s := newLegacyIDSorter(m.AllIds(), p.SortOptions.LegacySortOptions) + s := newLegacyIDSorter(m.Resources(), p.SortOptions.LegacySortOptions) sort.Sort(s) - err = applyOrdering(m, s.resids) - if err != nil { - return err - } - } - return nil -} -// applyOrdering takes resources (given in ResMap) and a desired ordering given -// as a sequence of ResIds, and updates the ResMap's resources to match the -// ordering. -func applyOrdering(m resmap.ResMap, ordering []resid.ResId) error { - var err error - resources := make([]*resource.Resource, m.Size()) - // Clear and refill with the correct order - for i, id := range ordering { - resources[i], err = m.GetByCurrentId(id) - if err != nil { - return errors.WrapPrefixf(err, "expected match for sorting") - } - } - m.Clear() - for _, r := range resources { - err = m.Append(r) - if err != nil { - return errors.WrapPrefixf(err, "SortOrderTransformer: Failed to append to resources") + // Clear the map and re-add the resources in the sorted order. + m.Clear() + for _, r := range s.resources { + err := m.Append(r) + if err != nil { + return errors.WrapPrefixf(err, "SortOrderTransformer: Failed to append to resources") + } } } return nil @@ -117,12 +99,17 @@ func applyOrdering(m resmap.ResMap, ordering []resid.ResId) error { type legacyIDSorter struct { // resids only stores the metadata of the object. This is an optimization as // it's expensive to compute these again and again during ordering. - resids []resid.ResId + resids []resid.ResId + // Initially, we sorted the metadata (ResId) of each object and then called GetByCurrentId on each to construct the final list. + // The problem is that GetByCurrentId is inefficient and does a linear scan in a list every time we do that. + // So instead, we sort resources alongside the ResIds. + resources []*resource.Resource + typeOrders map[string]int } func newLegacyIDSorter( - resids []resid.ResId, + resources []*resource.Resource, options *types.LegacySortOptions) *legacyIDSorter { // Precalculate a resource ranking based on the priority lists. var typeOrders = func() map[string]int { @@ -135,10 +122,13 @@ func newLegacyIDSorter( } return m }() - return &legacyIDSorter{ - resids: resids, - typeOrders: typeOrders, + + ret := &legacyIDSorter{typeOrders: typeOrders} + for _, res := range resources { + ret.resids = append(ret.resids, res.CurId()) + ret.resources = append(ret.resources, res) } + return ret } var _ sort.Interface = legacyIDSorter{} @@ -146,6 +136,7 @@ var _ sort.Interface = legacyIDSorter{} func (a legacyIDSorter) Len() int { return len(a.resids) } func (a legacyIDSorter) Swap(i, j int) { a.resids[i], a.resids[j] = a.resids[j], a.resids[i] + a.resources[i], a.resources[j] = a.resources[j], a.resources[i] } func (a legacyIDSorter) Less(i, j int) bool { if !a.resids[i].Gvk.Equals(a.resids[j].Gvk) { diff --git a/plugin/builtin/sortordertransformer/SortOrderTransformer.go b/plugin/builtin/sortordertransformer/SortOrderTransformer.go index fefc810d78..03b3e0b243 100644 --- a/plugin/builtin/sortordertransformer/SortOrderTransformer.go +++ b/plugin/builtin/sortordertransformer/SortOrderTransformer.go @@ -77,34 +77,16 @@ func (p *plugin) Transform(m resmap.ResMap) (err error) { // Sort if p.SortOptions.Order == types.LegacySortOrder { - s := newLegacyIDSorter(m.AllIds(), p.SortOptions.LegacySortOptions) + s := newLegacyIDSorter(m.Resources(), p.SortOptions.LegacySortOptions) sort.Sort(s) - err = applyOrdering(m, s.resids) - if err != nil { - return err - } - } - return nil -} -// applyOrdering takes resources (given in ResMap) and a desired ordering given -// as a sequence of ResIds, and updates the ResMap's resources to match the -// ordering. -func applyOrdering(m resmap.ResMap, ordering []resid.ResId) error { - var err error - resources := make([]*resource.Resource, m.Size()) - // Clear and refill with the correct order - for i, id := range ordering { - resources[i], err = m.GetByCurrentId(id) - if err != nil { - return errors.WrapPrefixf(err, "expected match for sorting") - } - } - m.Clear() - for _, r := range resources { - err = m.Append(r) - if err != nil { - return errors.WrapPrefixf(err, "SortOrderTransformer: Failed to append to resources") + // Clear the map and re-add the resources in the sorted order. + m.Clear() + for _, r := range s.resources { + err := m.Append(r) + if err != nil { + return errors.WrapPrefixf(err, "SortOrderTransformer: Failed to append to resources") + } } } return nil @@ -120,12 +102,17 @@ func applyOrdering(m resmap.ResMap, ordering []resid.ResId) error { type legacyIDSorter struct { // resids only stores the metadata of the object. This is an optimization as // it's expensive to compute these again and again during ordering. - resids []resid.ResId + resids []resid.ResId + // Initially, we sorted the metadata (ResId) of each object and then called GetByCurrentId on each to construct the final list. + // The problem is that GetByCurrentId is inefficient and does a linear scan in a list every time we do that. + // So instead, we sort resources alongside the ResIds. + resources []*resource.Resource + typeOrders map[string]int } func newLegacyIDSorter( - resids []resid.ResId, + resources []*resource.Resource, options *types.LegacySortOptions) *legacyIDSorter { // Precalculate a resource ranking based on the priority lists. var typeOrders = func() map[string]int { @@ -138,10 +125,13 @@ func newLegacyIDSorter( } return m }() - return &legacyIDSorter{ - resids: resids, - typeOrders: typeOrders, + + ret := &legacyIDSorter{typeOrders: typeOrders} + for _, res := range resources { + ret.resids = append(ret.resids, res.CurId()) + ret.resources = append(ret.resources, res) } + return ret } var _ sort.Interface = legacyIDSorter{} @@ -149,6 +139,7 @@ var _ sort.Interface = legacyIDSorter{} func (a legacyIDSorter) Len() int { return len(a.resids) } func (a legacyIDSorter) Swap(i, j int) { a.resids[i], a.resids[j] = a.resids[j], a.resids[i] + a.resources[i], a.resources[j] = a.resources[j], a.resources[i] } func (a legacyIDSorter) Less(i, j int) bool { if !a.resids[i].Gvk.Equals(a.resids[j].Gvk) {