-
Notifications
You must be signed in to change notification settings - Fork 967
Use HandleOrRuntime to allow alloydb/ethersdb to hold a custom runtime #1576
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 all commits
00adb4b
fda89ed
f94b1b7
ba20812
e6c8a57
01899bb
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 |
|---|---|---|
|
|
@@ -2,27 +2,29 @@ use std::sync::Arc; | |
|
|
||
| use ethers_core::types::{Block, BlockId, TxHash, H160 as eH160, H256, U64 as eU64}; | ||
| use ethers_providers::Middleware; | ||
| use tokio::runtime::Handle; | ||
| use tokio::runtime::{Handle, Runtime}; | ||
|
|
||
| use crate::primitives::{AccountInfo, Address, Bytecode, B256, U256}; | ||
| use crate::{Database, DatabaseRef}; | ||
|
|
||
| #[derive(Debug, Clone)] | ||
| use super::utils::HandleOrRuntime; | ||
|
|
||
| #[derive(Debug)] | ||
| pub struct EthersDB<M: Middleware> { | ||
| client: Arc<M>, | ||
| block_number: Option<BlockId>, | ||
| handle: Handle, | ||
| rt: HandleOrRuntime, | ||
| } | ||
|
|
||
| impl<M: Middleware> EthersDB<M> { | ||
| /// Create ethers db connector inputs are url and block on what we are basing our database (None for latest). | ||
| /// | ||
| /// Returns `None` if no tokio runtime is available or if the current runtime is a current-thread runtime. | ||
| pub fn new(client: Arc<M>, block_number: Option<BlockId>) -> Option<Self> { | ||
| let handle = match Handle::try_current() { | ||
| let rt = match Handle::try_current() { | ||
| Ok(handle) => match handle.runtime_flavor() { | ||
| tokio::runtime::RuntimeFlavor::CurrentThread => return None, | ||
| _ => handle, | ||
| _ => HandleOrRuntime::Handle(handle), | ||
| }, | ||
| Err(_) => return None, | ||
| }; | ||
|
|
@@ -31,29 +33,73 @@ impl<M: Middleware> EthersDB<M> { | |
| Some(Self { | ||
| client, | ||
| block_number, | ||
| handle, | ||
| rt, | ||
| }) | ||
| } else { | ||
| let mut instance = Self { | ||
| client: client.clone(), | ||
| client, | ||
| block_number: None, | ||
| handle, | ||
| rt, | ||
| }; | ||
| instance.block_number = Some(BlockId::from( | ||
| instance.block_on(client.get_block_number()).ok()?, | ||
| instance.block_on(instance.client.get_block_number()).ok()?, | ||
| )); | ||
| Some(instance) | ||
| } | ||
| } | ||
|
|
||
| /// Create a new EthersDB instance, with a provider and a block (None for latest) and a runtime. | ||
| /// | ||
| /// Refer to [tokio::runtime::Builder] how to create a runtime if you are in synchronous world. | ||
| /// If you are already using something like [tokio::main], call EthersDB::new instead. | ||
| pub fn with_runtime( | ||
| client: Arc<M>, | ||
| block_number: Option<BlockId>, | ||
| runtime: Runtime, | ||
|
Collaborator
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. so the caller is responsible for creating a new runtime?
Contributor
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. Make sense, we can add a new function and document the behavior. |
||
| ) -> Option<Self> { | ||
| let rt = HandleOrRuntime::Runtime(runtime); | ||
| let mut instance = Self { | ||
| client, | ||
| block_number, | ||
| rt, | ||
| }; | ||
|
|
||
| instance.block_number = Some(BlockId::from( | ||
| instance.block_on(instance.client.get_block_number()).ok()?, | ||
| )); | ||
| Some(instance) | ||
| } | ||
|
|
||
| /// Create a new EthersDB instance, with a provider and a block (None for latest) and a handle. | ||
| /// | ||
| /// This generally allows you to pass any valid runtime handle, refer to [tokio::runtime::Handle] on how | ||
| /// to obtain a handle. If you are already in asynchronous world, like [tokio::main], use EthersDB::new instead. | ||
| pub fn with_handle( | ||
| client: Arc<M>, | ||
| block_number: Option<BlockId>, | ||
| handle: Handle, | ||
| ) -> Option<Self> { | ||
| let rt = HandleOrRuntime::Handle(handle); | ||
| let mut instance = Self { | ||
| client, | ||
| block_number, | ||
| rt, | ||
| }; | ||
|
|
||
| instance.block_number = Some(BlockId::from( | ||
| instance.block_on(instance.client.get_block_number()).ok()?, | ||
| )); | ||
| Some(instance) | ||
| } | ||
|
|
||
| /// Internal utility function to call tokio feature and wait for output | ||
| #[inline] | ||
| fn block_on<F>(&self, f: F) -> F::Output | ||
| where | ||
| F: core::future::Future + Send, | ||
| F::Output: Send, | ||
| { | ||
| tokio::task::block_in_place(move || self.handle.block_on(f)) | ||
| self.rt.block_on(f) | ||
| } | ||
|
|
||
| /// set block number on which upcoming queries will be based | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| use tokio::runtime::{Handle, Runtime}; | ||
|
|
||
| // Hold a tokio runtime handle or full runtime | ||
| #[derive(Debug)] | ||
| pub(crate) enum HandleOrRuntime { | ||
| Handle(Handle), | ||
| Runtime(Runtime), | ||
| } | ||
|
|
||
| impl HandleOrRuntime { | ||
| #[inline] | ||
| pub(crate) fn block_on<F>(&self, f: F) -> F::Output | ||
| where | ||
| F: std::future::Future + Send, | ||
| F::Output: Send, | ||
| { | ||
| match self { | ||
| Self::Handle(handle) => tokio::task::block_in_place(move || handle.block_on(f)), | ||
| Self::Runtime(rt) => rt.block_on(f), | ||
|
Collaborator
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 will panic if called within async execution context
Contributor
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. Yes, that’s intended. Passing a runtime mostly would be the sync code. If user is within async execution context, they should call new directly instead.
Contributor
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. As stated above, if user is in async context, they should call new as #1557 suggests. If user is in sync context, they should provide a runtime, mostly current_thread runtime. If user intends to mix different contexts, they should be careful by themselves as it is known to be bad practice and cause problems hard to tackle. |
||
| } | ||
| } | ||
| } | ||
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.
@wtdcode would you explain why Clone has been removed? I'm slightly new in Rust and want to learn the technical concept behind this decision
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.
Because runtime doesn’t implement Clone and it doesn’t make too much sense cloning the db. Or, you could wrap it with an Rc or Arc.
What’s your use case?
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.
thanks
In my case, I define an ethersDB instance and by using it, I define a cacheDB. I add some base data (deploy contract, change some slots, etc) to cacheDB and after that, by cloning it, I send the same copies to multiple threads.
For now, I get an error (when cloning cacheDB) complains about ethersDB (as internal object of cacheDB) cannot be cloned.
because inside separate threads, I need to write to db, I tested not cloning and use Arc/Mutex. but the performance was worse than cloning.
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.
It's not really make sense to clone
AlloyDBorEthersDB.Clonewas also added by me in a previous PR, but I recently found it was a mistake. Generally, it is due to:ProviderandMiddlewareare only safe to clone within the same runtime. If you send it across runtimes (or threads), many internal things and assumptions could break.To your use case which is similar to mine, my suggestion is:
Arc<Mutex<CacheDB<EthersDB>>>across all threads. This avoids duplicate caching for each thread and can speed up your application overall.CacheDBmembers (note they are public) except theEthersDBand create a newEthersDBinstead. This generously is a manual "Clone" implementation.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.
thanks, I have a challenge in using arc.
in order to create evm and then transact(), I must do lock. but, about all of my perf. overhead is related to these steps, so it convert my program in a literally single-threaded system.
do you have any suggestion for this case?
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.
Note
with_dbaccepts borrows.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.
you mean
with_ref_db? I want to update the cache in eachtransactfrom multiple threads