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

feat(anvil): add ipc support #3134

Merged
merged 14 commits into from Sep 15, 2022
Merged

feat(anvil): add ipc support #3134

merged 14 commits into from Sep 15, 2022

Conversation

mattsse
Copy link
Member

@mattsse mattsse commented Sep 8, 2022

Motivation

Closes #2023

Solution

  • rename WsConnection -> PubSubConnection and abstract over Connection so it can be used by anything that's Stream+Sink
  • pull in ipc crate
  • Todo convert ipc connect (asyncread +asyncwrite) into a Stream+Sink
  • integrate in launch process
  • should be ready now, only needs some additional tests

@mattsse mattsse added the C-anvil Command: anvil label Sep 8, 2022
anvil/server/src/ipc.rs Outdated Show resolved Hide resolved
anvil/server/src/ipc.rs Outdated Show resolved Hide resolved
async fn on_request(&self, request: Self::Request, cx: PubSubContext<Self>) -> ResponseResult;
}

type Subscriptions<SubscriptionId, Subscription> = Arc<Mutex<Vec<(SubscriptionId, Subscription)>>>;
Copy link
Member

Choose a reason for hiding this comment

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

Why a vec and not a map of subid to subscriptions?

Copy link
Member Author

Choose a reason for hiding this comment

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

this is a vec so we can easily poll them one in round robinish fashion
polling also requires pinning which is easier when you have mut self and are not constrained by working with a &mut ref of a map

let mut subscriptions = pin.context.subscriptions.lock();
'outer: for n in (0..subscriptions.len()).rev() {
let (id, mut sub) = subscriptions.swap_remove(n);

lookup is only done when adding/removing subscriptions so the overhead is quite small

anvil/src/config.rs Outdated Show resolved Hide resolved
}
}

struct JsonRpcCodec;
Copy link
Member

Choose a reason for hiding this comment

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

Are we OK with writing our own codecs? Is this something we should start refactoring, eg for have in reth?

Copy link
Member Author

Choose a reason for hiding this comment

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

the current state of the different codec traits and AsyncReadWrite + Stream/Sink is a bit of a mess...

we probably only need this when we something that's only AsyncWrite/AsyncRead but we need a stream,

The way this currently works is Framed::new(asyncread+asyncwrite, codec) the codec here determines when a new framed object can be produced as Stream::Item

this codec basically finds enclosed json objects via { and determines when a full object is in the buffer and can be deserialized.

@cyberthirst
Copy link

sweet! thanks for this feature. I measured circa 40% performance boost when using ipc over websockets. (using web3py)

iFrostizz pushed a commit to iFrostizz/foundry that referenced this pull request Nov 9, 2022
* refactor: rename pubusb

* feat: add ipc support

* feat: impl IpcConn

* docs

* feat: integrate ipc service

* fix: use futures ready

* feat: more ipc impls

* fix: always flush

* typos instrument
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-anvil Command: anvil
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ipc support for anvil
3 participants