perf: cache analysed bytecodes

[dinhani-cw]

No description provided.

[github-actions]

PR Review

⏱️ Estimated effort to review [1-5]	3, because the PR involves changes to caching mechanisms and bytecode analysis which are critical for performance and correctness in the context of Ethereum virtual machines. The changes are not extensive but require a good understanding of the existing system and the implications of caching.
🧪 Relevant tests	No
🔍 Possible issues	Possible Bug: The caching mechanism does not handle potential concurrency issues. If multiple threads access bytecode_cache, there could be race conditions unless concurrency control is implemented.
	Performance Concern: Repeated cloning of analysed_bytecode could lead to performance degradation, especially if the bytecode is large.
🔒 Security concerns	No

Code feedback:

---

relevant file	src/eth/evm/revm.rs
suggestion	Consider implementing concurrency control for the bytecode_cache to prevent race conditions in a multi-threaded environment. This might involve using a Mutex or another thread-safe data structure to wrap the HashMap. [important]
relevant line	bytecode_cache: HashMap,

---

relevant file	src/eth/evm/revm.rs
suggestion	To reduce memory usage and potential performance hits from cloning large bytecodes, consider storing references or using a reference-counted pointer like Arc instead of cloning the bytecode. [important]
relevant line	self.bytecode_cache.insert(address, analysed_bytecode.clone());

---

---

✨ Review tool usage guide:

---

Overview:
The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

• When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:

/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

• With a configuration file, use the following template:

[pr_reviewer]
some_config1=...
some_config2=...

See the review usage page for a comprehensive guide on using this tool.

[github-actions]

PR Code Suggestions

Category	Suggestions
Performance	Improve performance by avoiding unnecessary data cloning. Consider using a reference to raw_bytecode when calling to_analysed to avoid unnecessary data cloning if to_analysed can accept a reference. src/eth/evm/revm.rs [261] -let analysed_bytecode = to_analysed(raw_bytecode); +let analysed_bytecode = to_analysed(&raw_bytecode);
	Optimize the performance of the hashmap by using a faster hashing algorithm. Ensure that the bytecode_cache HashMap uses a suitable hashing algorithm for performance, especially if Address has a simple byte structure. src/eth/evm/revm.rs [197] -bytecode_cache: HashMap<Address, RevmBytecode>, +bytecode_cache: HashMap<Address, RevmBytecode, BuildHasherDefault<FnvHasher>>,
	Use Arc for efficient memory management and to avoid cloning large data structures. To avoid cloning the bytecode for the cache, consider using an Arc to manage shared ownership efficiently. src/eth/evm/revm.rs [262-263] +let analysed_bytecode = Arc::new(to_analysed(raw_bytecode)); self.bytecode_cache.insert(address, analysed_bytecode.clone()); revm_account.code = Some(analysed_bytecode)
Enhancement	Simplify and optimize caching logic using or_insert_with. Use or_insert_with to handle caching logic more idiomatically and efficiently by only computing the analysed bytecode when it is not already cached. src/eth/evm/revm.rs [256-263] -match self.bytecode_cache.get(&address) { - Some(analysed_bytecode) => revm_account.code = Some(analysed_bytecode.clone()), - None => { - let analysed_bytecode = to_analysed(raw_bytecode); - self.bytecode_cache.insert(address, analysed_bytecode.clone()); - revm_account.code = Some(analysed_bytecode) - } -} +let analysed_bytecode = self.bytecode_cache.entry(address) + .or_insert_with(|| to_analysed(raw_bytecode)) + .clone(); +revm_account.code = Some(analysed_bytecode);
Robustness	Add error handling for bytecode analysis to improve robustness. Consider handling potential errors from to_analysed function gracefully, especially if the bytecode analysis can fail. src/eth/evm/revm.rs [261] -let analysed_bytecode = to_analysed(raw_bytecode); +let analysed_bytecode = to_analysed(raw_bytecode).unwrap_or_else(|e| { + log::error!("Failed to analyse bytecode: {}", e); + RevmBytecode::default() +});

---

✨ Improve tool usage guide:

---

Overview:
The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

• When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:

/improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...

• With a configuration file, use the following template:

[pr_code_suggestions]
some_config1=...
some_config2=...

See the improve usage page for a comprehensive guide on using this tool.

[dinhani-cw]

The logic is fine, but E2E fails because tests that resets the blockchain are not resetting the bytecode cache.

So the best way to have this cache is in the storage layer and not inside the REVM.