diff --git a/src/pipelines/exec/navi.rs b/src/pipelines/exec/navi.rs index 099823b..a4ec31e 100644 --- a/src/pipelines/exec/navi.rs +++ b/src/pipelines/exec/navi.rs @@ -25,7 +25,7 @@ use { pub(crate) struct StepPath(SmallVec<[usize; 8]>); const PROLOGUE_INDEX: usize = usize::MIN; -const EPILOGUE_START_INDEX: usize = usize::MAX - 1024; // Reserve space for epilogue steps +const EPILOGUE_INDEX: usize = usize::MAX; const STEP0_INDEX: usize = PROLOGUE_INDEX + 1; /// Public API @@ -86,7 +86,7 @@ impl StepPath { /// Returns `true` if the path is pointing to an epilogue of a pipeline. pub(crate) fn is_epilogue(&self) -> bool { - self.leaf() >= EPILOGUE_START_INDEX + self.leaf() == EPILOGUE_INDEX } } @@ -232,15 +232,9 @@ impl StepPath { Self(smallvec![PROLOGUE_INDEX]) } - /// Returns a leaf step path pointing at the first epilogue step. + /// Returns a leaf step path pointing at the epilogue step. pub(in crate::pipelines) fn epilogue() -> Self { - Self::epilogue_step(0) - } - - /// Returns a leaf step path pointing at a specific epilogue step with the - /// given index. - pub(in crate::pipelines) fn epilogue_step(epilogue_index: usize) -> Self { - Self(smallvec![epilogue_index + EPILOGUE_START_INDEX]) + Self(smallvec![EPILOGUE_INDEX]) } /// Returns a new step path that points to the first non-prologue step. @@ -281,18 +275,14 @@ impl core::fmt::Display for StepPath { if let Some(&first) = iter.next() { match first { PROLOGUE_INDEX => write!(f, "p"), - idx if idx >= EPILOGUE_START_INDEX => { - write!(f, "e{}", idx - EPILOGUE_START_INDEX) - } + EPILOGUE_INDEX => write!(f, "e"), index => write!(f, "{index}"), }?; for &index in iter { match index { PROLOGUE_INDEX => write!(f, "_p"), - idx if idx >= EPILOGUE_START_INDEX => { - write!(f, "_e{}", idx - EPILOGUE_START_INDEX) - } + EPILOGUE_INDEX => write!(f, "_e"), index => write!(f, "_{index}"), }?; } @@ -328,7 +318,7 @@ impl<'a, P: Platform> StepNavigator<'a, P> { /// In pipelines with a prologue, this will point to the prologue step. /// In pipelines without a prologue, this will point to the first step. /// In pipelines with no steps, but with an epilogue, this will point to the - /// first epilogue step. + /// epilogue step. /// /// If the first item in the pipeline is a nested pipeline, this will dig /// deeper into the nested pipeline to find the first executable item. @@ -346,10 +336,9 @@ impl<'a, P: Platform> StepNavigator<'a, P> { // pipeline has no prologue if pipeline.steps().is_empty() { - // If there are no steps, but there is an epilogue, return the first - // epilogue step. - if !pipeline.epilogue().is_empty() { - return Self(StepPath::epilogue(), vec![pipeline]).enter(); + // If there are no steps, but there is an epilogue, return it. + if pipeline.epilogue().is_some() { + return Some(Self(StepPath::epilogue(), vec![pipeline])); } // this is an empty pipeline, there is nothing executable. @@ -373,20 +362,11 @@ impl<'a, P: Platform> StepNavigator<'a, P> { } else if self.is_epilogue() { enclosing_pipeline .epilogue() - .get( - step_index - .checked_sub(EPILOGUE_START_INDEX) - .expect("step index should be >= epilogue start index"), - ) .expect("Step path points to a non-existing epilogue") } else { let StepOrPipeline::Step(step) = enclosing_pipeline .steps() - .get( - step_index - .checked_sub(STEP0_INDEX) - .expect("step index should be >= step0 index"), - ) + .get(step_index - STEP0_INDEX) .expect("Step path points to a non-existing step") else { unreachable!( @@ -420,19 +400,7 @@ impl<'a, P: Platform> StepNavigator<'a, P> { /// Returns `None` if there are no more steps to execute in the pipeline. pub(crate) fn next_ok(self) -> Option { if self.is_epilogue() { - // we are in an epilogue step, check if there are more epilogue steps - let enclosing_pipeline = self.pipeline(); - let epilogue_index = self - .0 - .leaf() - .checked_sub(EPILOGUE_START_INDEX) - .expect("invalid epilogue step index in the step path"); - - if epilogue_index + 1 < enclosing_pipeline.epilogue().len() { - // there are more epilogue steps, go to the next one - return Self(self.0.increment_leaf(), self.1.clone()).enter(); - } - // this is the last epilogue step, we are done with this pipeline + // the loop is over. return self.next_in_parent(); } @@ -551,56 +519,45 @@ impl StepNavigator<'_, P> { "StepNavigator should always have at least one enclosing pipeline", ); - if path.is_prologue() { + if path.is_prologue() || path.is_epilogue() { assert!( - enclosing_pipeline.prologue().is_some(), - "path is prologue, but the enclosing pipeline has none", + enclosing_pipeline.prologue().is_some() + || enclosing_pipeline.epilogue().is_some(), + "path is prologue or epilogue, but enclosing pipeline has none", ); - // if we are in a prologue, we can just return ourselves. + // if we are in a prologue or epilogue, we can just return ourselves. return Some(Self(path, ancestors)); } - if path.is_epilogue() { - assert!( - !enclosing_pipeline.epilogue().is_empty(), - "path is epilogue, but the enclosing pipeline has none", - ); - Some(Self(path, ancestors)) - } else { - let step_index = path - .leaf() - .checked_sub(STEP0_INDEX) - .expect("path is not prologue or epilogue"); - - match enclosing_pipeline.steps().get(step_index)? { - StepOrPipeline::Step(_) => { - // if we are pointing at a step, we can just return ourselves. - Some(Self(path, ancestors)) - } - StepOrPipeline::Pipeline(_, nested) => { - // if we are pointing at a pipeline, we need to dig into its - // entrypoint. - Some(StepNavigator(path, ancestors).join(Self::entrypoint(nested)?)) - } + let step_index = path + .leaf() + .checked_sub(STEP0_INDEX) + .expect("path is not prologue or epilogue"); + + match enclosing_pipeline.steps().get(step_index)? { + StepOrPipeline::Step(_) => { + // if we are pointing at a step, we can just return ourselves. + Some(Self(path, ancestors)) + } + StepOrPipeline::Pipeline(_, nested) => { + // if we are pointing at a pipeline, we need to dig into its + // entrypoint. + Some(StepNavigator(path, ancestors).join(Self::entrypoint(nested)?)) } } } /// Finds the next step to run when a loop is finished. /// - /// The next step could be either the first epilogue step of the current + /// The next step could be either the epilogue step of the current /// pipeline or the next step in the parent pipeline. fn after_loop(self) -> Option { - if self.pipeline().epilogue().is_empty() { - self.next_in_parent() + if self.pipeline().epilogue().is_some() { + // we've reached the epilogue of this pipeline, regardless of the + // looping behavior, we should go to the next step in the parent pipeline. + Some(Self(self.0.replace_leaf(EPILOGUE_INDEX), self.1.clone())) } else { - // we've reached the epilogue of this pipeline, go to the first epilogue - // step - Some(Self( - self.0.replace_leaf(EPILOGUE_START_INDEX), - self.1.clone(), - )) - .and_then(|nav| nav.enter()) + self.next_in_parent() } } @@ -667,9 +624,7 @@ mod test { fake_step!(Epilogue1); fake_step!(Epilogue2); - - fake_step!(EpilogueStep1); - fake_step!(EpilogueStep2); + fake_step!(Epilogue3); fake_step!(Prologue1); fake_step!(Prologue2); @@ -711,8 +666,7 @@ mod test { Once, (StepI, StepII, StepIII) .with_name("nested1.2") - .with_epilogue(EpilogueStep1) - .with_epilogue(EpilogueStep2), + .with_epilogue(Epilogue3), ) }) .with_step(Step4) @@ -728,10 +682,6 @@ mod test { self.concat(StepPath::epilogue()) } - fn append_epilogue_step(self, step_index: usize) -> Self { - self.concat(StepPath::epilogue_step(step_index)) - } - fn append_step(self, step_index: usize) -> Self { self.concat(StepPath::step(step_index)) } @@ -755,7 +705,7 @@ mod test { }}; } - // one step + // one step with no prologue assert_entrypoint!( Pipeline::::default().with_step(Step1), StepPath::step0(), @@ -772,7 +722,7 @@ mod test { vec!["one"] ); - // no steps, but with a one-step epilogue + // no steps, but with epilogue assert_entrypoint!( Pipeline::::named("one").with_epilogue(Epilogue1), StepPath::epilogue(), @@ -797,7 +747,7 @@ mod test { vec!["one", "two"] ); - // one nested pipeline with no steps, but with a one-step epilogue + // one nested pipeline with no steps, but with an epilogue assert_entrypoint!( Pipeline::::named("one") .with_pipeline(Loop, Pipeline::named("two").with_epilogue(Epilogue1)), @@ -935,13 +885,7 @@ mod test { assert_eq!(cursor.0, StepPath::step(2).append_step(3).append_step(1)); let cursor = cursor.next_break().unwrap(); - assert_eq!( - cursor.0, - StepPath::step(2).append_step(3).append_epilogue_step(0) - ); - - let cursor = cursor.next_ok().unwrap(); - StepPath::step(2).append_step(3).append_epilogue_step(1); + assert_eq!(cursor.0, StepPath::step(2).append_step(3).append_epilogue()); let cursor = cursor.next_ok().unwrap(); assert_eq!(cursor.0, StepPath::step(2).append_step(0)); diff --git a/src/pipelines/iter.rs b/src/pipelines/iter.rs index b688362..7fb950d 100644 --- a/src/pipelines/iter.rs +++ b/src/pipelines/iter.rs @@ -11,7 +11,7 @@ struct Frame<'a, P: Platform> { path: StepPath, next_ix: usize, yielded_prologue: bool, - epilogue_ix: usize, + yielded_epilogue: bool, } impl<'a, P: Platform> StepPathIter<'a, P> { @@ -22,7 +22,7 @@ impl<'a, P: Platform> StepPathIter<'a, P> { next_ix: 0, path: StepPath::empty(), yielded_prologue: false, - epilogue_ix: 0, + yielded_epilogue: false, }], } } @@ -56,18 +56,17 @@ impl Iterator for StepPathIter<'_, P> { path: next_path, next_ix: 0, yielded_prologue: false, - epilogue_ix: 0, + yielded_epilogue: false, }); continue; } } } - // Walk epilogue steps/pipelines; descend into nested pipelines. - if frame.epilogue_ix < frame.pipeline.epilogue.len() { - let ix = frame.epilogue_ix; - frame.epilogue_ix += 1; - return Some(frame.path.clone().concat(StepPath::epilogue_step(ix))); + // Yield epilogue once, if present. + if !frame.yielded_epilogue && frame.pipeline.epilogue.is_some() { + frame.yielded_epilogue = true; + return Some(frame.path.clone().concat(StepPath::epilogue())); } // Done with this frame; pop and continue with parent. @@ -103,7 +102,7 @@ mod tests { .with_step(Step3) .with_epilogue(EpilogueOne); - let expected = vec!["p", "1", "2", "3", "e0"]; + let expected = vec!["p", "1", "2", "3", "e"]; let actual = pipeline .iter_steps() .map(|step| step.to_string()) @@ -126,7 +125,7 @@ mod tests { .with_epilogue(EpilogueOne); let expected = vec![ - "p", "1_p", "1_1", "1_2_1", "1_2_2", "1_2_3", "1_2_e0", "1_3", "e0", + "p", "1_p", "1_1", "1_2_1", "1_2_2", "1_2_3", "1_2_e", "1_3", "e", ]; let actual = pipeline diff --git a/src/pipelines/mod.rs b/src/pipelines/mod.rs index 2b4b915..8eb9e39 100644 --- a/src/pipelines/mod.rs +++ b/src/pipelines/mod.rs @@ -40,7 +40,7 @@ pub enum Behavior { } pub struct Pipeline { - epilogue: Vec>>, + epilogue: Option>>, prologue: Option>>, steps: Vec>, limits: Option>>, @@ -53,7 +53,7 @@ impl Default for Pipeline

{ #[track_caller] fn default() -> Self { Self { - epilogue: Vec::new(), + epilogue: None, prologue: None, steps: Vec::new(), limits: None, @@ -88,12 +88,11 @@ impl Pipeline

{ } /// A step that happens as the last step of the block after the whole payload - /// has been built. Can be called multiple times to add multiple epilogue - /// steps. + /// has been built. #[must_use] pub fn with_epilogue(self, step: impl Step

) -> Self { let mut this = self; - this.epilogue.push(Arc::new(StepInstance::new(step))); + this.epilogue = Some(Arc::new(StepInstance::new(step))); this } @@ -170,7 +169,7 @@ impl Pipeline

{ impl Pipeline

{ /// Returns true if the pipeline has no steps, prologue or epilogue. pub fn is_empty(&self) -> bool { - self.prologue.is_none() && self.epilogue.is_empty() && self.steps.is_empty() + self.prologue.is_none() && self.epilogue.is_none() && self.steps.is_empty() } /// An optional name of the pipeline. @@ -195,8 +194,8 @@ impl Pipeline

{ self.prologue.as_ref() } - pub(crate) fn epilogue(&self) -> &[Arc>] { - &self.epilogue + pub(crate) fn epilogue(&self) -> Option<&Arc>> { + self.epilogue.as_ref() } pub(crate) fn steps(&self) -> &[StepOrPipeline

] { @@ -334,7 +333,7 @@ impl core::fmt::Debug for Pipeline

{ f.debug_struct("Pipeline") .field("name", &self.name()) .field("prologue", &self.prologue.as_ref().map(|p| p.name())) - .field("epilogue", &self.epilogue) + .field("epilogue", &self.epilogue.as_ref().map(|p| p.name())) .field("steps", &self.steps) .field("limits", &self.limits.is_some()) .finish_non_exhaustive()