-
Notifications
You must be signed in to change notification settings - Fork 12
feat: mutliple prologue steps #44
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
Open
julio4
wants to merge
1
commit into
flashbots:main
Choose a base branch
from
julio4:feat/multiple-prologue-steps
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,7 @@ use { | |
| core::fmt::Formatter, | ||
| derive_more::{From, Into}, | ||
| smallvec::{SmallVec, smallvec}, | ||
| std::sync::Arc, | ||
| std::{cmp::min, sync::Arc}, | ||
| }; | ||
|
|
||
| /// Represents a path to a step or a nested pipeline in a pipeline. | ||
|
|
@@ -24,9 +24,10 @@ use { | |
| #[derive(PartialEq, Eq, Clone, From, Into, Hash)] | ||
| pub(crate) struct StepPath(SmallVec<[usize; 8]>); | ||
|
|
||
| const EXTRA_SECTIONS_SIZE: usize = 1024; | ||
| const PROLOGUE_INDEX: usize = usize::MIN; | ||
| const EPILOGUE_START_INDEX: usize = usize::MAX - 1024; // Reserve space for epilogue steps | ||
| const STEP0_INDEX: usize = PROLOGUE_INDEX + 1; | ||
| const STEP0_INDEX: usize = PROLOGUE_INDEX + EXTRA_SECTIONS_SIZE + 1; | ||
| const EPILOGUE_START_INDEX: usize = usize::MAX - EXTRA_SECTIONS_SIZE; | ||
|
|
||
| /// Public API | ||
| impl StepPath { | ||
|
|
@@ -81,7 +82,7 @@ impl StepPath { | |
|
|
||
| /// Returns `true` if the path is pointing to a prologue of a pipeline. | ||
| pub(crate) fn is_prologue(&self) -> bool { | ||
| self.leaf() == PROLOGUE_INDEX | ||
| self.leaf() < STEP0_INDEX | ||
| } | ||
|
|
||
| /// Returns `true` if the path is pointing to an epilogue of a pipeline. | ||
|
|
@@ -227,30 +228,44 @@ impl StepPath { | |
| /// | ||
| /// None of those methods do any checks on the validity of the path. | ||
| impl StepPath { | ||
| /// Returns a step path that points to the prologue step. | ||
| pub(in crate::pipelines) fn prologue() -> Self { | ||
| Self(smallvec![PROLOGUE_INDEX]) | ||
| /// Returns a leaf step path pointing at a specific prologue step with the | ||
| /// given index. | ||
| pub(in crate::pipelines) fn prologue_step(prologue_index: usize) -> Self { | ||
| Self(smallvec![min( | ||
| prologue_index + PROLOGUE_INDEX, | ||
| STEP0_INDEX - 1, | ||
| )]) | ||
| } | ||
|
|
||
| /// Returns a leaf step path pointing at the first epilogue step. | ||
| pub(in crate::pipelines) fn epilogue() -> Self { | ||
| Self::epilogue_step(0) | ||
| /// Returns a step path that points to the first prologue step. | ||
| pub(in crate::pipelines) fn prologue() -> Self { | ||
| Self::prologue_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.saturating_add(EPILOGUE_START_INDEX) | ||
| ]) | ||
| } | ||
|
|
||
| /// Returns a new step path that points to the first non-prologue step. | ||
| pub(in crate::pipelines) fn step0() -> Self { | ||
| Self::step(0) | ||
| /// Returns a leaf step path pointing at the first epilogue step. | ||
| pub(in crate::pipelines) fn epilogue() -> Self { | ||
| Self::epilogue_step(0) | ||
| } | ||
|
|
||
| /// Returns a leaf step path pointing at a step with the given index. | ||
| pub(in crate::pipelines) fn step(step_index: usize) -> Self { | ||
| Self(smallvec![step_index + STEP0_INDEX]) | ||
| Self(smallvec![min( | ||
| step_index + STEP0_INDEX, | ||
| EPILOGUE_START_INDEX - 1 | ||
| )]) | ||
| } | ||
|
|
||
| /// Returns a new step path that points to the first non-prologue step. | ||
| pub(in crate::pipelines) fn step0() -> Self { | ||
| Self::step(0) | ||
| } | ||
|
|
||
| /// Appends a new path to the current path. | ||
|
|
@@ -280,20 +295,22 @@ impl core::fmt::Display for StepPath { | |
|
|
||
| if let Some(&first) = iter.next() { | ||
| match first { | ||
| PROLOGUE_INDEX => write!(f, "p"), | ||
| idx if idx < STEP0_INDEX => write!(f, "p{}", idx - PROLOGUE_INDEX), | ||
| idx if idx >= EPILOGUE_START_INDEX => { | ||
| write!(f, "e{}", idx - EPILOGUE_START_INDEX) | ||
| } | ||
| index => write!(f, "{index}"), | ||
| idx => write!(f, "{}", idx - STEP0_INDEX), | ||
| }?; | ||
|
|
||
| for &index in iter { | ||
| match index { | ||
| PROLOGUE_INDEX => write!(f, "_p"), | ||
| idx if idx < STEP0_INDEX => { | ||
| write!(f, "_p{}", idx - PROLOGUE_INDEX) | ||
| } | ||
| idx if idx >= EPILOGUE_START_INDEX => { | ||
| write!(f, "_e{}", idx - EPILOGUE_START_INDEX) | ||
| } | ||
| index => write!(f, "_{index}"), | ||
| idx => write!(f, "_{}", idx - STEP0_INDEX), | ||
| }?; | ||
| } | ||
| } | ||
|
|
@@ -325,7 +342,7 @@ impl<'a, P: Platform> StepNavigator<'a, P> { | |
| /// Given a pipeline, returns a navigator that points at the first executable | ||
| /// item in the pipeline. | ||
| /// | ||
| /// In pipelines with a prologue, this will point to the prologue step. | ||
| /// In pipelines with a prologue, this will point to the first 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. | ||
|
|
@@ -340,8 +357,8 @@ impl<'a, P: Platform> StepNavigator<'a, P> { | |
| } | ||
|
|
||
| // pipeline has a prologue, return it. | ||
| if pipeline.prologue().is_some() { | ||
| return Some(Self(StepPath::prologue(), vec![pipeline])); | ||
| if !pipeline.prologue().is_empty() { | ||
| return Self(StepPath::prologue(), vec![pipeline]).enter(); | ||
| } | ||
|
|
||
| // pipeline has no prologue | ||
|
|
@@ -369,6 +386,11 @@ impl<'a, P: Platform> StepNavigator<'a, P> { | |
| if self.is_prologue() { | ||
| enclosing_pipeline | ||
| .prologue() | ||
| .get( | ||
| step_index | ||
| .checked_sub(PROLOGUE_INDEX) | ||
| .expect("step index should be >= prologue index"), | ||
| ) | ||
| .expect("Step path points to a non-existing prologue") | ||
| } else if self.is_epilogue() { | ||
| enclosing_pipeline | ||
|
|
@@ -419,9 +441,10 @@ 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<Self> { | ||
| let enclosing_pipeline = self.pipeline(); | ||
|
|
||
| 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() | ||
|
|
@@ -437,12 +460,22 @@ impl<'a, P: Platform> StepNavigator<'a, P> { | |
| } | ||
|
|
||
| if self.is_prologue() { | ||
| // start looping (if possible) | ||
| // we are in a prologue step, check if there are more prologue steps | ||
| let prologue_index = self | ||
| .0 | ||
| .leaf() | ||
| .checked_sub(PROLOGUE_INDEX) | ||
julio4 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| .expect("invalid prologue step index in the step path"); | ||
|
|
||
| if prologue_index + 1 < enclosing_pipeline.prologue.len() { | ||
| // there are more prologue step, go to the next one | ||
| return Self(self.0.increment_leaf(), self.1.clone()).enter(); | ||
| } | ||
|
|
||
| // this is the last prologue step, enter step section | ||
| return self.after_prologue(); | ||
| } | ||
|
|
||
| let enclosing_pipeline = self.pipeline(); | ||
|
|
||
| // we are in a regular step. | ||
| assert!( | ||
| !enclosing_pipeline.steps().is_empty(), | ||
|
|
@@ -480,7 +513,8 @@ impl<'a, P: Platform> StepNavigator<'a, P> { | |
| /// | ||
| /// Returns `None` if there are no more steps to execute in the pipeline. | ||
| pub(crate) fn next_break(self) -> Option<Self> { | ||
| if self.is_epilogue() { | ||
| // breaking in prologue or epilogue directly stop pipeline execution | ||
| if self.is_epilogue() || self.is_prologue() { | ||
|
Comment on lines
+516
to
+517
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can discuss this as well. When we break within a prologue, should we break out of the pipeline, or just go to pipeline steps? I believe breaking out makes more sense |
||
| // the loop is over. | ||
| return self.next_in_parent(); | ||
| } | ||
|
|
@@ -553,7 +587,7 @@ impl<P: Platform> StepNavigator<'_, P> { | |
|
|
||
| if path.is_prologue() { | ||
| assert!( | ||
| enclosing_pipeline.prologue().is_some(), | ||
| !enclosing_pipeline.prologue().is_empty(), | ||
| "path is prologue, but the enclosing pipeline has none", | ||
| ); | ||
| // if we are in a prologue, we can just return ourselves. | ||
|
|
@@ -673,6 +707,7 @@ mod test { | |
|
|
||
| fake_step!(Prologue1); | ||
| fake_step!(Prologue2); | ||
| fake_step!(Prologue3); | ||
|
|
||
| fake_step!(Step1); | ||
| fake_step!(Step2); | ||
|
|
@@ -704,6 +739,7 @@ mod test { | |
| (StepX, StepY, StepZ) | ||
| .with_name("nested1.1") | ||
| .with_prologue(Prologue2) | ||
| .with_prologue(Prologue3) | ||
| .with_epilogue(Epilogue2), | ||
| ) | ||
| .with_step(StepC) | ||
|
|
@@ -724,6 +760,10 @@ mod test { | |
| self.concat(StepPath::prologue()) | ||
| } | ||
|
|
||
| fn append_prologue_step(self, step_index: usize) -> Self { | ||
| self.concat(StepPath::prologue_step(step_index)) | ||
| } | ||
|
|
||
| fn append_epilogue(self) -> Self { | ||
| self.concat(StepPath::epilogue()) | ||
| } | ||
|
|
@@ -787,7 +827,7 @@ mod test { | |
| vec!["one", "two"] | ||
| ); | ||
|
|
||
| // one nested step with prologue | ||
| // one nested step with a one-step prologue | ||
| assert_entrypoint!( | ||
| Pipeline::<Ethereum>::named("one").with_pipeline( | ||
| Loop, | ||
|
|
@@ -905,7 +945,16 @@ mod test { | |
| assert_eq!(cursor.0, StepPath::step(2).append_step(0)); | ||
|
|
||
| let cursor = cursor.next_ok().unwrap(); | ||
| assert_eq!(cursor.0, StepPath::step(2).append_step(1).append_prologue()); | ||
| assert_eq!( | ||
| cursor.0, | ||
| StepPath::step(2).append_step(1).append_prologue_step(0) | ||
| ); | ||
|
|
||
| let cursor = cursor.next_ok().unwrap(); | ||
| assert_eq!( | ||
| cursor.0, | ||
| StepPath::step(2).append_step(1).append_prologue_step(1) | ||
| ); | ||
|
|
||
| let cursor = cursor.next_ok().unwrap(); | ||
| assert_eq!(cursor.0, StepPath::step(2).append_step(1).append_step(0)); | ||
|
|
@@ -947,12 +996,12 @@ mod test { | |
| assert_eq!(cursor.0, StepPath::step(2).append_step(0)); | ||
|
|
||
| let cursor = cursor.next_ok().unwrap(); | ||
| assert_eq!(cursor.0, StepPath::step(2).append_step(1).append_prologue()); | ||
| assert_eq!( | ||
| cursor.0, | ||
| StepPath::step(2).append_step(1).append_prologue_step(0) | ||
| ); | ||
|
|
||
| let cursor = cursor.next_break().unwrap(); | ||
| assert_eq!(cursor.0, StepPath::step(2).append_step(1).append_epilogue()); | ||
|
|
||
| let cursor = cursor.next_ok().unwrap(); | ||
| assert_eq!(cursor.0, StepPath::step(2).append_step(2)); | ||
|
|
||
| let cursor = cursor.next_break().unwrap(); | ||
|
|
@@ -998,12 +1047,15 @@ mod test { | |
| assert_eq!(navigator.instance().name(), cursor.instance().name()); | ||
|
|
||
| let cursor = cursor.next_ok().unwrap(); | ||
| assert_eq!(cursor.0, StepPath::step(2).append_step(1).append_prologue()); | ||
| assert_eq!( | ||
| cursor.0, | ||
| StepPath::step(2).append_step(1).append_prologue_step(0) | ||
| ); | ||
| let navigator = cursor.0.navigator(&pipeline).unwrap(); | ||
| assert_eq!(navigator.1.len(), cursor.1.len()); | ||
| assert_eq!( | ||
| navigator.0, | ||
| StepPath::step(2).append_step(1).append_prologue() | ||
| StepPath::step(2).append_step(1).append_prologue_step(0) | ||
| ); | ||
| assert_eq!(navigator.instance().name(), cursor.instance().name()); | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so we remove 0?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is basically useless as
PROLOGUE_INDEX = usize::MINbut it makes the whole logic correct (else we should just remove PROLOGUE_INDEX altogether). It should be optimized away at compilation anyway.