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.

Sign up for GitHub

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(pipeline): Improve execution times for dense pipeline graphs #4824

Merged
merged 11 commits into from
Jan 17, 2025
+644 −34
Merged

perf(pipeline): Improve execution times for dense pipeline graphs #4824

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
87fdd27
test(pipeline): Define StartStageHandler performance
Jan 6, 2025
862d3c6
fix(pipeline): Memoize anyUpstreamStagesFailed results
Jan 7, 2025
0cf61bb
refactor(stage): Move anyUpstreamStagesFailed to StartStageHandler
Jan 7, 2025
188ad02
fix(stage): Avoid using a ConcurrentHashMap
Jan 8, 2025
dd92e9c
docs(test): Use a more concise test name
Jan 8, 2025
e16c801
perf(stage): First check if a stage has been visited
Jan 9, 2025
4633ccf
perf(stage): Filter out non-synthetic stages
Jan 9, 2025
388dc75
perf(stage): Precompute requisiteStageRefIds
Jan 10, 2025
5a6775e
perf(stage): Only use withAuth when needed
Jan 10, 2025
4ac447f
perf(stage): Remove duplicate call to withStage
Jan 10, 2025
72fb2bf
chore(import): Clean up unused imports
Jan 10, 2025
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
7 changes: 5 additions & 2 deletions ...core/src/main/java/com/netflix/spinnaker/orca/pipeline/model/StageExecutionInternals.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.netflix.spinnaker.orca.api.pipeline.SyntheticStageOwner;
import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Set;

Expand All @@ -41,15 +42,17 @@ static List<StageExecution> getAncestorsImpl(
StageExecution stage, Set<String> visited, boolean directParentOnly) {
visited.add(stage.getRefId());

if (!stage.getRequisiteStageRefIds().isEmpty() && !directParentOnly) {
if (!directParentOnly && !stage.getRequisiteStageRefIds().isEmpty()) {
// Get stages this stage depends on via requisiteStageRefIds:
Collection<String> requisiteStageRefIds = stage.getRequisiteStageRefIds();
List<StageExecution> previousStages =
stage.getExecution().getStages().stream()
.filter(it -> stage.getRequisiteStageRefIds().contains(it.getRefId()))
.filter(it -> !visited.contains(it.getRefId()))
.filter(it -> requisiteStageRefIds.contains(it.getRefId()))
.collect(toList());
List<StageExecution> syntheticStages =
stage.getExecution().getStages().stream()
.filter(s -> s.getSyntheticStageOwner() != null)
Copy link
Member

Choose a reason for hiding this comment

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

Was it really going through all the stages before and not only the STAGE_BEFORE/AFTER?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes :(

Copy link
Member

Choose a reason for hiding this comment

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

That is a good catch 🚀 👏

.filter(
s ->
previousStages.stream()
Expand Down
4 changes: 0 additions & 4 deletions orca-kotlin/src/main/kotlin/com/netflix/spinnaker/orca/ext/Stage.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import com.netflix.spinnaker.orca.api.pipeline.SyntheticStageOwner.STAGE_BEFORE
import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus.CANCELED
import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus.FAILED_CONTINUE
import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus.NOT_STARTED
import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus.SKIPPED
import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus.STOPPED
import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus.SUCCEEDED
Expand Down Expand Up @@ -83,9 +82,6 @@ fun StageExecution.upstreamStages(): List<StageExecution> =
fun StageExecution.allUpstreamStagesComplete(): Boolean =
upstreamStages().all { it.status in listOf(SUCCEEDED, FAILED_CONTINUE, SKIPPED) }

fun StageExecution.anyUpstreamStagesFailed(): Boolean =
upstreamStages().any { it.status in listOf(TERMINAL, STOPPED, CANCELED) || it.status == NOT_STARTED && it.anyUpstreamStagesFailed() }

fun StageExecution.syntheticStages(): List<StageExecution> =
execution.stages.filter { it.parentStageId == id }

Expand Down
Loading
Loading