From 9939cf527eaadece243d0c5c75e78de478f1c556 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Fri, 19 Jun 2020 14:29:48 -0400 Subject: [PATCH] Change JobSteps to a List, Change Register Step function name --- src/Runner.Worker/ExecutionContext.cs | 68 +++---------------- .../Handlers/CompositeActionHandler.cs | 8 +-- src/Runner.Worker/JobRunner.cs | 2 +- src/Runner.Worker/StepsRunner.cs | 7 +- 4 files changed, 20 insertions(+), 65 deletions(-) diff --git a/src/Runner.Worker/ExecutionContext.cs b/src/Runner.Worker/ExecutionContext.cs index b99ff99cd..228074e6c 100644 --- a/src/Runner.Worker/ExecutionContext.cs +++ b/src/Runner.Worker/ExecutionContext.cs @@ -63,7 +63,7 @@ namespace GitHub.Runner.Worker JobContext JobContext { get; } // Only job level ExecutionContext has JobSteps - Queue JobSteps { get; } + List JobSteps { get; } // Only job level ExecutionContext has PostJobSteps Stack PostJobSteps { get; } @@ -105,8 +105,7 @@ namespace GitHub.Runner.Worker // others void ForceTaskComplete(); void RegisterPostJobStep(IStep step); - IStep RegisterCompositeStep(IStep step, DictionaryContextData inputsData); - void EnqueueAllCompositeSteps(Queue steps); + void RegisterNestedStep(IStep step, DictionaryContextData inputsData, int location); } public sealed class ExecutionContext : RunnerService, IExecutionContext @@ -161,7 +160,7 @@ namespace GitHub.Runner.Worker public List ServiceContainers { get; private set; } // Only job level ExecutionContext has JobSteps - public Queue JobSteps { get; private set; } + public List JobSteps { get; private set; } // Only job level ExecutionContext has PostJobSteps public Stack PostJobSteps { get; private set; } @@ -267,62 +266,17 @@ namespace GitHub.Runner.Worker Root.PostJobSteps.Push(step); } - /* - RegisterCompositeStep is a helper function used in CompositeActionHandler::RunAsync to - add a child node, aka a step, to the current job to the front of the queue for processing. - */ - public IStep RegisterCompositeStep(IStep step, DictionaryContextData inputsData) + /// + /// Helper function used in CompositeActionHandler::RunAsync to + /// add a child node, aka a step, to the current job to the Root.JobSteps based on the location. + /// + public void RegisterNestedStep(IStep step, DictionaryContextData inputsData, int location) { - // ~Brute Force Method~ - // There is no way to put this current job in front of the queue in < O(n) time where n = # of elements in JobSteps - // Everytime we add a new step, you could requeue every item to put those steps from that stack in JobSteps which - // would result in O(n) for each time we add a composite action step where n = number of jobSteps which would compound - // to O(n*m) where m = number of composite steps - // var temp = Root.JobSteps.ToArray(); - // Root.JobSteps.Clear(); - // Root.JobSteps.Enqueue(step); - // foreach(var s in temp) - // Root.JobSteps.Enqueue(s); - - // ~Optimized Method~ - // Alterative solution: We add to another temp Queue - // After we add all the transformed composite steps to this temp queue, we requeue the whole JobSteps accordingly in EnqueueAllCompositeSteps() - // where the queued composite steps are at the front of the JobSteps Queue and the rest of the jobs maintain its order and are - // placed after the queued composite steps - // This will take only O(n+m) time which would be just as efficient if we refactored the code of JobSteps to a PriorityQueue - // This temp Queue is created in the CompositeActionHandler. + // TODO: For UI purposes, look at figuring out how to condense steps in one node => maybe use the same previous GUID var newGuid = Guid.NewGuid(); step.ExecutionContext = Root.CreateChild(newGuid, step.DisplayName, newGuid.ToString("N"), null, null); step.ExecutionContext.ExpressionValues["inputs"] = inputsData; - return step; - } - - // Add Composite Steps first and then requeue the rest of the job steps. - public void EnqueueAllCompositeSteps(Queue steps) - { - // TODO: For UI purposes, look at figuring out how to condense steps in one node - // maybe use "this" instead of "Root"? - if (Root.JobSteps != null) - { - var temp = Root.JobSteps.ToArray(); - Root.JobSteps.Clear(); - foreach (var cs in steps) - { - Root.JobSteps.Enqueue(cs); - } - foreach (var s in temp) - { - Root.JobSteps.Enqueue(s); - } - } - else - { - Root.JobSteps = new Queue(); - foreach (var cs in steps) - { - Root.JobSteps.Enqueue(cs); - } - } + Root.JobSteps.Insert(0, step); } public IExecutionContext CreateChild(Guid recordId, string displayName, string refName, string scopeName, string contextName, Dictionary intraActionState = null, int? recordOrder = null) @@ -719,7 +673,7 @@ namespace GitHub.Runner.Worker PrependPath = new List(); // JobSteps for job ExecutionContext - JobSteps = new Queue(); + JobSteps = new List(); // PostJobSteps for job ExecutionContext PostJobSteps = new Stack(); diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index c29e6c10f..74c3d4c7e 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -45,7 +45,7 @@ namespace GitHub.Runner.Worker.Handlers } // Add each composite action step to the front of the queue - var compositeActionSteps = new Queue(); + int location = 0; foreach (Pipelines.ActionStep aStep in actionSteps) { // Ex: @@ -63,7 +63,7 @@ namespace GitHub.Runner.Worker.Handlers // - run echo hello world 3 (d) // - run echo hello world 4 (e) // - // Stack (LIFO) [Bottom => Middle => Top]: + // Steps processed as follow: // | a | // | a | => | d | // (Run step d) @@ -86,9 +86,9 @@ namespace GitHub.Runner.Worker.Handlers // TODO: Do we need to add any context data from the job message? // (See JobExtension.cs ~line 236) - compositeActionSteps.Enqueue(ExecutionContext.RegisterCompositeStep(actionRunner, inputsData)); + ExecutionContext.RegisterNestedStep(actionRunner, inputsData, location); + location++; } - ExecutionContext.EnqueueAllCompositeSteps(compositeActionSteps); return Task.CompletedTask; } diff --git a/src/Runner.Worker/JobRunner.cs b/src/Runner.Worker/JobRunner.cs index 31dfb1714..33b291adb 100644 --- a/src/Runner.Worker/JobRunner.cs +++ b/src/Runner.Worker/JobRunner.cs @@ -152,7 +152,7 @@ namespace GitHub.Runner.Worker { foreach (var step in jobSteps) { - jobContext.JobSteps.Enqueue(step); + jobContext.JobSteps.Add(step); } await stepsRunner.RunAsync(jobContext); diff --git a/src/Runner.Worker/StepsRunner.cs b/src/Runner.Worker/StepsRunner.cs index 3c7b38d4a..9fc323c86 100644 --- a/src/Runner.Worker/StepsRunner.cs +++ b/src/Runner.Worker/StepsRunner.cs @@ -59,14 +59,15 @@ namespace GitHub.Runner.Worker checkPostJobActions = true; while (jobContext.PostJobSteps.TryPop(out var postStep)) { - jobContext.JobSteps.Enqueue(postStep); + jobContext.JobSteps.Add(postStep); } continue; } - var step = jobContext.JobSteps.Dequeue(); - var nextStep = jobContext.JobSteps.Count > 0 ? jobContext.JobSteps.Peek() : null; + var step = jobContext.JobSteps[0]; + jobContext.JobSteps.RemoveAt(0); + var nextStep = jobContext.JobSteps.Count > 0 ? jobContext.JobSteps[0] : null; // TODO: Fix this temporary workaround for Composite Actions if (!string.IsNullOrEmpty(Environment.GetEnvironmentVariable("TESTING_COMPOSITE_ACTIONS_ALPHA"))) {