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

perf: improve applyOrdering by avoid call to GetByCurrentId #5079

Merged
merged 1 commit into from
Apr 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 22 additions & 31 deletions api/internal/builtins/SortOrderTransformer.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

53 changes: 22 additions & 31 deletions plugin/builtin/sortordertransformer/SortOrderTransformer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -138,17 +125,21 @@ 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{}

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) {
Expand Down
Loading