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(core): avoid nested looping in execution transformation #7217

Merged
merged 5 commits into from
Aug 14, 2019
Merged

perf(core): avoid nested looping in execution transformation #7217

merged 5 commits into from
Aug 14, 2019

Conversation

anotherchrisberry
Copy link
Contributor

We do a lot of find and filter calls inside the applyPhasesAndLink method, and some of them are nested, resulting in a lot of unnecessary lookups.

Pretty small refactor here: create a dictionary of stages with the refId as the key. Then use that.

Also, since applyPhasesAndLink is recursive, I'm pulling some logic that only needs to happen once (deduping the requisiteStageRefId values) out of that method to make sure it only happens once.

The impact here is decent? For our biggest application, rendering 10 executions per pipeline, we save about a second of JS execution (2s -> 1s); for 50/pipeline, it's about three seconds (6.5s -> 3.6s). This is dwarfed by the time it takes to download the 50 executions! But probably still worth doing, as I don't think it makes this code any more complex or harder to grok.

Copy link
Contributor

@maggieneterval maggieneterval left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome!

@Jammy-Louie Jammy-Louie merged commit 1e779ef into spinnaker:master Aug 14, 2019
christopherthielen added a commit to christopherthielen/deck that referenced this pull request Aug 14, 2019
598b5b3 refactor(hooks): Pulling out useData (spinnaker#7325)
0fbafe6 fix(core/pipeline): Refresh jenkins jobs component when manual execution trigger is changed (spinnaker#7323)
1251159 fix(core/pipeline): make dropdown option labels always be strings (spinnaker#7322)
1e779ef perf(core): avoid nested looping in execution transformation (spinnaker#7217)
bec54df refactor(core): Reactify the pipeline trigger configuration (spinnaker#7318)
christopherthielen added a commit that referenced this pull request Aug 14, 2019
598b5b3 refactor(hooks): Pulling out useData (#7325)
0fbafe6 fix(core/pipeline): Refresh jenkins jobs component when manual execution trigger is changed (#7323)
1251159 fix(core/pipeline): make dropdown option labels always be strings (#7322)
1e779ef perf(core): avoid nested looping in execution transformation (#7217)
bec54df refactor(core): Reactify the pipeline trigger configuration (#7318)
@anotherchrisberry anotherchrisberry deleted the ext-perf branch January 8, 2020 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants