-
Notifications
You must be signed in to change notification settings - Fork 30
[WIP] feat: initial implementation of pull-streams based multiplexer #76
Conversation
25aa583
to
9615da1
Compare
NOTE: I've added pull-mplex as a git dep, so that we can test with it across different dependencies. This is temporary and should be changed once pull-mplex is released. |
Pull Request Test Coverage Report for Build 216
💛 - Coveralls |
I want to preserve both implementations and make it easy to switch between both and have benchmarks in this repo. This translates to:
|
Ok, most of this is already in pull-mplex and can/should be moved. There are integration tests for the previous and new versions, as well as benchmarks, also this already runs against interface-stream-muxer tests. I'll move them over to this repo. |
Sounds good! Thank you :) |
45537f8
to
62b5446
Compare
benchmarks include: - multiplex - pull-mplex - libp2p-mplex (and libp2p-mplex using pull-mplex) License: MIT Signed-off-by: Dmitriy Ryajov <dryajov@gmail.com>
3e066d3
to
af3f60f
Compare
@dryajov What's the status of this pr? Is it going to be continued? Is it superseded by |
yes it is, but being that pull-streams are being replaced in favor of async iterators, I wonder if it makes sense anymore. |
@dryajov Also wouldn't it make sense to have this much effort in production at least for a couple weeks? Better late than never ;) |
@mkg20001 I agree, I would very much love to see this in prod, but I doubt I'll be able to finish it at this point or there is time from the rest of the team to review it, plus the move to async, might make the pull-stream approach obsolete altogether. Although, it might be a good benchmark to compare against along size node streams. As a start tho, can we get a preliminary review of |
For the reader:
|
the multiplexer itself is being implemented as a separate module under libp2p/pull-mplex#1. This is done for reusability purposes
but it should be moved under the libp2p org at some point.