-
Notifications
You must be signed in to change notification settings - Fork 111
perf(levm): store valid jump targets with code #4961
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
Changes from 17 commits
8796743
d3e55ee
a2ed233
639bd66
8ba26c6
94247b6
cd630b8
7beda28
3a7a105
17fcc1a
39b3c00
407c831
200282e
6f0efd4
22608f1
50ffa3b
09594d3
f8e6c25
0537c13
6c92879
0bb059f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,11 +19,58 @@ use crate::{ | |
| utils::keccak, | ||
| }; | ||
|
|
||
| #[allow(unused)] | ||
| #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] | ||
| pub struct Code { | ||
| pub hash: H256, | ||
| pub bytecode: Bytes, | ||
| // TODO: Consider using Arc<u16> (needs to enable serde rc feature) | ||
| pub jump_targets: Vec<u16>, | ||
edg-l marked this conversation as resolved.
Show resolved
Hide resolved
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. This needs a comment explaining the reason for using |
||
| } | ||
|
|
||
| impl Code { | ||
| // TODO: also add `from_hashed_bytecode` to optimize the download pipeline, | ||
| // where hash is already known and checked. | ||
| pub fn from_bytecode(code: Bytes) -> Self { | ||
| let jump_targets = Self::compute_jump_targets(&code); | ||
| Self { | ||
| hash: keccak(code.as_ref()), | ||
| bytecode: code, | ||
| jump_targets, | ||
| } | ||
| } | ||
|
|
||
| fn compute_jump_targets(code: &[u8]) -> Vec<u16> { | ||
| debug_assert!(code.len() <= u16::MAX as usize); | ||
|
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. This needs a comment |
||
| let mut targets = Vec::new(); | ||
| let mut i = 0; | ||
| while i < code.len() { | ||
| match code[i] { | ||
| // OP_JUMPDEST | ||
| 0x5B => { | ||
| targets.push(i as u16); | ||
| } | ||
| // OP_PUSH1..32 | ||
| c @ 0x60..0x80 => { | ||
| i += (c - 0x5F) as usize; | ||
| } | ||
| _ => (), | ||
| } | ||
| i += 1; | ||
| } | ||
| targets | ||
| } | ||
| } | ||
|
|
||
| impl AsRef<Bytes> for Code { | ||
| fn as_ref(&self) -> &Bytes { | ||
| &self.bytecode | ||
| } | ||
| } | ||
|
|
||
| #[derive(Clone, Default, Debug, PartialEq, Eq, Serialize, Deserialize)] | ||
| pub struct Account { | ||
| pub info: AccountInfo, | ||
| pub code: Bytes, | ||
| pub code: Code, | ||
| pub storage: BTreeMap<H256, U256>, | ||
| } | ||
|
|
||
|
|
@@ -63,6 +110,16 @@ impl Default for AccountState { | |
| } | ||
| } | ||
|
|
||
| impl Default for Code { | ||
| fn default() -> Self { | ||
| Self { | ||
| bytecode: Bytes::new(), | ||
| hash: *EMPTY_KECCACK_HASH, | ||
| jump_targets: Vec::new(), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl From<GenesisAccount> for Account { | ||
| fn from(genesis: GenesisAccount) -> Self { | ||
| Self { | ||
|
|
@@ -71,7 +128,7 @@ impl From<GenesisAccount> for Account { | |
| balance: genesis.balance, | ||
| nonce: genesis.nonce, | ||
| }, | ||
| code: genesis.code, | ||
| code: Code::from_bytecode(genesis.code), | ||
| storage: genesis | ||
| .storage | ||
| .iter() | ||
|
|
@@ -160,11 +217,11 @@ impl From<&GenesisAccount> for AccountState { | |
| } | ||
|
|
||
| impl Account { | ||
| pub fn new(balance: U256, code: Bytes, nonce: u64, storage: BTreeMap<H256, U256>) -> Self { | ||
| pub fn new(balance: U256, code: Code, nonce: u64, storage: BTreeMap<H256, U256>) -> Self { | ||
| Self { | ||
| info: AccountInfo { | ||
| balance, | ||
| code_hash: keccak(code.as_ref()).0.into(), | ||
| code_hash: code.hash, | ||
| nonce, | ||
| }, | ||
| code, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -194,8 +194,14 @@ pub fn stateless_validation_l2( | |
|
|
||
| // Check state diffs are valid | ||
| let blob_versioned_hash = if !validium { | ||
| use bytes::Bytes; | ||
| use ethrex_common::types::Code; | ||
|
|
||
| let mut guest_program_state = GuestProgramState { | ||
| codes_hashed, | ||
| codes_hashed: codes_hashed | ||
| .into_iter() | ||
| .map(|(h, c)| (h, Code::from_bytecode(Bytes::from_owner(c)))) | ||
| .collect(), | ||
| parent_block_header, | ||
| first_block_number: initial_db.first_block_number, | ||
| chain_config: initial_db.chain_config, | ||
|
|
@@ -274,7 +280,11 @@ fn execute_stateless( | |
| #[cfg(feature = "l2")] | ||
| let nodes_hashed = guest_program_state.nodes_hashed.clone(); | ||
| #[cfg(feature = "l2")] | ||
| let codes_hashed = guest_program_state.codes_hashed.clone(); | ||
| let codes_hashed = guest_program_state | ||
| .codes_hashed | ||
| .iter() | ||
| .map(|(h, c)| (*h, c.bytecode.to_vec())) | ||
| .collect(); | ||
|
Comment on lines
+288
to
+292
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. Many of these conversions seem unneeded. We should check that they don't affect the performance of the L2. |
||
| #[cfg(feature = "l2")] | ||
| let parent_block_header_clone = guest_program_state.parent_block_header.clone(); | ||
|
|
||
|
|
||
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.
The command was using removedb as a setup for each batch of runs, but the command was cancelled due to having no input. I changed it to use
--prepareinstead, which runs before each single run, and pipedyesinto it to automatically confirm.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 was reverted, but we should fix this if we intend to keep this benchmark.