Skip to content
This repository has been archived by the owner on Jul 21, 2023. It is now read-only.

[WIP] pull-multiplex - pull-streams all the way down #44

Closed
wants to merge 7 commits into from

Conversation

dignifiedquire
Copy link
Member

@dignifiedquire dignifiedquire commented Dec 25, 2016

Current State

  • New multiplex implementation using only pull-streams in src/multiplex.js
    • Spec compliant
      • Do not encode the header with a length prefix
      • Check maximum size of data
    • finish destroy method
    • All tests passing
      • Mega stress test, does not run out of memory, but doesn't finish either.
      • Close tests
    • Split into their own package
      • src/pull-end
      • src/pull-split
      • src/multiplex
  • Cleanup code
  • https://github.com/libp2p/interface-stream-muxer/compare/fix/multiplex?expand=1
  • Add documentation

@daviddias
Copy link
Member

Have you tested this with the stress tests? Any luck?

@dignifiedquire
Copy link
Member Author

dignifiedquire commented Dec 25, 2016

@diasdavid

@@ -0,0 +1,39 @@
/* eslint-env mocha */
Copy link
Member

Choose a reason for hiding this comment

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

Not sure how this file here is relevant, seems to be a demonstration of promises.

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 super important :P no I just forgot to delete it before comitting, fell free to remove

@victorb
Copy link
Member

victorb commented Jan 20, 2017

There is an updated branch based on this work but by using multiplex from npm rather than a re-implementation of the core of multiplex here: #49

@daviddias daviddias changed the title [WIP] Convert to pull stream [WIP] pull-multiplex - pull-streams all the way down Jan 20, 2017
@victorb
Copy link
Member

victorb commented Jan 20, 2017

pull-wrap branch was merged in 0f3bc8c and exposes a pull-stream interface identical to the one here, but with using multiplex module instead of our implementation.

We'll keep this one open for the future.

@daviddias daviddias closed this Dec 13, 2017
@daviddias daviddias deleted the pull-stream branch December 13, 2017 07:18
@ghost ghost removed the status/in-progress In progress label Dec 13, 2017
@daviddias
Copy link
Member

@dryajov now that you are attempting #73, make sure to go through the work @dignifiedquire did here

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

Successfully merging this pull request may close these issues.

3 participants