-
Notifications
You must be signed in to change notification settings - Fork 2
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: built-in libp2p node #48
Conversation
5f75ca7
to
fd91c2c
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
The PR is ready for review 🙏🏻 @juliangruber Please focus on especially on the JavaScript bits (the public API) @tchardin @mxinden if you have the bandwidth to review the libp2p side, then I am happy to hear your feedback! You can find the libp2p bits in /cc @patrickwoodhead in case you were interested |
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
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.
Nice work! As far as I understand it, lgtm!
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
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 for the ping. Great to see rust-libp2p in action here.
Let me know in case I can be of any help.
Also let me know in case you need help upgrading to the next release (v0.51.0
).
|
||
use super::behaviour::RequestId; | ||
|
||
// FIXME: Can we use `[u8]` instead? How to avoid cloning when sending the data between threads? |
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.
Where is the clone happening? You could consider adding a lifetime to RequestProtocol
. Also you could consider using buffer pools. All that said, I wouldn't introduce such optimizations without previous benchmarks showing that this is an actual bottleneck.
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.
For request payloads:
-
The JS engine allocates
Uint8Array
on the GC-managed heap -
Our
op
receives this value as aZeroCopyBuf
. Quoting from Deno API docs:A ZeroCopyBuf encapsulates a slice that's been borrowed from a JavaScript ArrayBuffer object. JavaScript objects can normally be garbage collected, but the existence of a ZeroCopyBuf inhibits this until it is dropped. It behaves much like an Arc<u8>.
I think we may be able to use
&[u8]
instead ofZeroCopyBuf
in ourop
function, but I am not sure.The concept is the same though - we get a reference to a slice pointing into the memory managed by the JS runtime.
Now that I am writing this, I got an idea: maybe I can change
RequestPayload
to become an alias forZeroCopyBuf
and see how that works. -
Our
op
converts that slice intoVec<u8>
and that's where to clone/copy happens.Line 77 in bc2712a
request_payload.to_vec(),
A somewhat-related discussion in Deno that may shed more light on this topic: denoland/deno#4788
For response payloads:
-
We allocate
Vec<u8>
so we have a place to read the response payload into.
zinnia/ext/libp2p/peer/protocol.rs
Lines 72 to 73 in bc2712a
let mut response: ResponsePayload = Default::default(); io.take(10 * 1024 * 1024).read_to_end(&mut response).await?; -
The
op
returns this value to Deno runtime. The runtime allocates a newUint8Array
on the GC-managed heap and copies the data there.
I want to rework this part very soon (see #56) to allow the JavaScript side to repeatedly read a chunk of data from the sub stream into a buffer provided by the JavaScript side (a.k.a BYOB). I expect that work to require changes to the ResponsePayload
; thus I am not too worried about this response part.
@mxinden Thank you for the offer! I'll reach out in case of issues I am not able to resolve myself. BTW, I was expecting Dependabot to open a pull request to upgrade libp2p to the next release. Do you happen to know why it did not and/or how can we change our config to fix that? I suspect this is related to Dependabot's config option
In our workspace, we have both an app (cli/Cargo.toml) and several libraries (e.g. ext/libp2p/Cargo.toml and runtime/Cargo.toml). Ideally, I want to build the CLI with the latest versions of dependencies but keep our libraries supporting a range of older versions that are compatible with the latest. Do you have any suggestions or recommendations on specifying our dependency version ranges and configuring Dependabot to achieve that? |
As far as I can tell, you are only pointing dependabot to https://github.com/mxinden/libp2p-perf/blob/master/.github/dependabot.yml |
See #9
Quoting from the documentation:
Zinnia comes with a built-in libp2p node based on rust-libp2p. The node is shared by all Station Modules running on Zinnia. This way we can keep the number of open connections lower and avoid duplicate entries in routing tables.
The initial version comes with a limited subset of features. We will be adding more features based on feedback from our users (Station Module builders).
Networking stack
tcp
using system DNS resolvernoise
withXX
handshake pattern using X25519 DH keysyamux
andmplex
Zinnia.peerId
Type:
string
Return the peer id of Zinnia's built-in libp2p peer. The peer id is ephemeral, Zinnia generates a new peer id every time it starts.
Zinnia.requestProtocol(remoteAddress, protocolName, requestPayload)
This PR adds a built-in libp2p node based on rust-libp2p. The node is shared by
all Station Modules running on Zinnia. This way, we can lower the number of
open connections and avoid duplicate entries in DHTs.
Networking stack
tcp
using system DNS resolvernoise
withXX
handshake pattern using X25519 DH keysyamux
andmplex
Zinnia.peerId
Type:
string
Return the peer id of Zinnia's built-in libp2p peer. The peer id is ephemeral,
Zinnia generates a new peer id every time it starts.
Zinnia.requestProtocol(remoteAddress, protocolName, requestPayload)
Dial a remote peer identified by the
remoteAddress
and open a new substream for the protocol identified byprotocolName
. SendrequestPayload
and read the response payload.The function returns a promise that resolves with a readable-stream-like object. At the moment, this object implements "async iterable" protocol only, it's not a full readable stream. This is enough to allow you to receive response in chunks, where each chunk is an
Uint8Array
instance.Notes:
Example