Merge pull request #5079 from chlunde/perf-1

perf: improve applyOrdering by avoid call to GetByCurrentId

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
+		// Clear the map and re-add the resources in the sorted order.
-// as a sequence of ResIds, and updates the ResMap's resources to match the
+		m.Clear()
-// ordering.
+		for _, r := range s.resources {
-func applyOrdering(m resmap.ResMap, ordering []resid.ResId) error {
+			err := m.Append(r)
-	var err error
+			if err != nil {
-	resources := make([]*resource.Resource, m.Size())
+				return errors.WrapPrefixf(err, "SortOrderTransformer: Failed to append to resources")
-	// 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")
 		}
 	}
 	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,
+	ret := &legacyIDSorter{typeOrders: typeOrders}
-		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) {

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
+		// Clear the map and re-add the resources in the sorted order.
-// as a sequence of ResIds, and updates the ResMap's resources to match the
+		m.Clear()
-// ordering.
+		for _, r := range s.resources {
-func applyOrdering(m resmap.ResMap, ordering []resid.ResId) error {
+			err := m.Append(r)
-	var err error
+			if err != nil {
-	resources := make([]*resource.Resource, m.Size())
+				return errors.WrapPrefixf(err, "SortOrderTransformer: Failed to append to resources")
-	// 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")
 		}
 	}
 	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,
+	ret := &legacyIDSorter{typeOrders: typeOrders}
-		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) {