Skip to content
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

use oneshot channel to bridge async and blocking-io tasks, this can avoid blocking task block async runtime #3093

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Pana
Copy link
Member

@Pana Pana commented Feb 25, 2025

Currently, all underlying RPC handlers are essentially synchronous methods, which involve blocking I/O operations. This affects task scheduling in the asynchronous runtime, leading to performance degradation. This PR introduces a bridge that correctly spawns blocking I/O operations using spawn_blocking and sends the results back via a oneshot channel after task completion.

This PR will solve issue #3077

This change is Reviewable

@Pana Pana changed the title use oneshot channel to avoid blocking task block async runtime use oneshot channel to bridge async and block tasks, this can avoid blocking task block async runtime Feb 25, 2025
@Pana
Copy link
Member Author

Pana commented Feb 25, 2025

retest this please

@Pana
Copy link
Member Author

Pana commented Feb 25, 2025

retest this please

@Pana Pana requested a review from ChenxingLi February 26, 2025 00:31
@Pana Pana changed the title use oneshot channel to bridge async and block tasks, this can avoid blocking task block async runtime use oneshot channel to bridge async and blocking-io tasks, this can avoid blocking task block async runtime Feb 26, 2025
Copy link
Contributor

@ChenxingLi ChenxingLi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 20 of 22 files at r1, all commit messages.
Reviewable status: 20 of 22 files reviewed, 1 unresolved discussion


crates/rpc/rpc-eth-impl/src/eth.rs line 1068 at r1 (raw file):

    {
        let self_clone = self.clone();
        async move {

Why there are nested async closures?

Copy link
Member Author

@Pana Pana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 20 of 22 files reviewed, 1 unresolved discussion (waiting on @ChenxingLi)


crates/rpc/rpc-eth-impl/src/eth.rs line 1068 at r1 (raw file):

Previously, ChenxingLi (Chenxing Li) wrote…

Why there are nested async closures?

It is required to use async closures to capture the cloned self, otherwise there will be a lifetime issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants