-
Notifications
You must be signed in to change notification settings - Fork 506
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
Why not having an async Database trait? #1575
Comments
The new API style may be similar to |
Appreciate if async db trait feature can be done. I used to fork and modify it to async for v3.5.0. And it's more difficult to do the same thing for the latest version. |
|
this can be optimized by adding another variant that does not use block in place for every call. but it's unreasonable to make entire evm execution async hence there's no async variant |
I understand your solution because I once also tried to dedicate futures to standalone runtimes, which can also be achieved by my previous PR. It works and generally should be the intended solution as suggested by tokio documents.
I agree that evm exution is sync by nature but the db access is not, no? This also reminds me of another solution: how about other async executors instead of tokio? Since we only need to spin and block in the |
The interpreter is very sensitive to these kinds of changes and VM work is not that small that I would be okay to put it in a async task. |
Reading through #554 and #1534 , I notice that the root cause is that current
revm
doesn't have async traits.Given the fact that we already split the original single
Database
trait into bothDatabase
andDatabaseRef
traits, can we have another trait likeDatabaseAsync
(and maybeDatabaseRefAsync
) by using https://github.com/dtolnay/async-trait ? In this case:Database
immediately getDatabaseAsync
implemented by simply forwarding function calls. However, implementingDatabaseAsync
doesn't implyDatabase
trait.basic
isasync
now. And they will no longer implementDatabase
.Database
implementation play with async world?Generally, the modification is a bit huge because we need to add new
async
API sincetransact()
and many places to support the async trait. Thus, I'm opening the issue for discussion before starting to work on it.The text was updated successfully, but these errors were encountered: