simple-peer icon indicating copy to clipboard operation
simple-peer copied to clipboard

Thoughts on reducing browser bundle size

Open nazar-pc opened this issue 6 years ago • 22 comments

In my current app simple-peer is about 10% of an application size and about 20% of JS code size. Specifically, while index.js is 29.5KiB and one might expect that minified version would be even smaller, simplepeer.min.js is whopping 86.5KiB right now.

Before I create and start maintaining a fork of this project, I'd like to know if if this is something that can be fixed upstream.

2 packages that cause this huge size:

  • readable-stream
  • buffer

readable-stream is valuable in Node.js, but not so much in browser environment. Is it possible that future versions of simple-peer will stop using it entirely and replace with something lightweight instead? I imagine it wouldn't be too hard to implement a wrapper or even offer an optional one with this library.

buffer is a simpler one, once readable-stream is eliminated we'll just need to switch from Buffer to Uint8Array, which is quite easy to do.

Current bundle size doesn't seem tolerable to me given that it is just a wrapper around native features, so I hope for your understanding here. If these changes seem reasonable, I'd be happy to implement them myself, otherwise I'll be forced to create something like simple-peer-light and just maintain that fork forever.

nazar-pc avatar Apr 11 '18 21:04 nazar-pc

In fact, I went ahead and removed streaming functionality, converted Buffer to Uint8Array and got 26.7KiB of minified build. Could be even more savings (about 19KiB) when debug module is removed, but even without that an impressive result I think. https://github.com/feross/simple-peer/compare/master...Detox:8bf7e98e5f354d8f1804d07c63b55dcd2dc9fa46?expand=1

nazar-pc avatar Apr 11 '18 22:04 nazar-pc

Do you think it would be possible to retain the stream API by making a smaller readable-stream alternative? simple-peer is pretty entrenched in the "stream module" ecosystem.

I think debug could be outright removed, seeing as it's only being used as a logger and not using any of debug's unique features.

t-mullen avatar Apr 12 '18 22:04 t-mullen

I'm not sure any alternative would be much smaller. If streaming API support is claimed, the whole API must be supported.

One option might be to provide a shim for browser environments that will not contain actual implementation, but will have a few things required by simple-peer itself. Otherwise some kind of decoupling would be necessary in order to build browser version with different set of pluggable components.

Another option might be to use minimal shim by default and accept duplex stream constructor as optional argument in simple-peer to upgrade it when needed. It wouldn't be difficult to supply it in Node.js.

nazar-pc avatar Apr 12 '18 22:04 nazar-pc

We could release two separate bundles (simple-peer and simple-peer-lite), the decoupling wouldn't be too difficult.

The lite version could be built without those larger modules and use a non-streaming send API. Replacing a module with an empty stub is fairly simple with browserify and can be detected in the constructor to determine which send method to expose. That way we have a single codebase and preserve the existing features.

t-mullen avatar Apr 12 '18 22:04 t-mullen

Problems with removing it:

  • breaking change, all webtorrent packages and many dependents would need to get rewritten
  • lots of tests are written against the stream api

We could make a minimal shim and include it in this repo so users could require('simple-peer/lite') but I don't want to include a whole second set of tests for the lite version.

This seems like a lot of work.

feross avatar Apr 12 '18 22:04 feross

I'll experiment with shim and let you know how it goes

nazar-pc avatar Apr 12 '18 22:04 nazar-pc

If the cost isn't so high to maintaining the lite version in this repo, I'm open to it. Send a PR when you have something to look at @nazar-pc

feross avatar Apr 12 '18 22:04 feross

Could we somehow split core features from streaming and offer streaming version in index.js while having non-streaming Buffer-free version in simplepeer.min.js. Those who need streaming in browser will be able to use Browserify as usual and everyone should be happy.

nazar-pc avatar May 26 '18 01:05 nazar-pc

Maybe we should wait until we abstract the DataChannel for multiplexing support? It might make it easier to replace Buffer/Stream dependencies.

t-mullen avatar Jun 12 '18 22:06 t-mullen

I don't see them as being related. Basically I expect that "light" version would only implement data event and with Uint8Array instead of Buffer as the replacement for streaming functionality. Other events and APIs should be the same.

nazar-pc avatar Jun 12 '18 23:06 nazar-pc

My thought was you could pass a lite version of the DataChannel wrapper that uses UInt8Array and does not have a streaming API instead of changing anything about the peer object.

t-mullen avatar Jun 12 '18 23:06 t-mullen

But streaming is a part of peer object's API. Could you elaborate on your idea?

nazar-pc avatar Jun 12 '18 23:06 nazar-pc

Sorry, yes. For multiplexing the plan is to have two objects: A Peer that handles signaling, media, etc, and a DataChannelWrapper that is a Duplex stream. The peer exposes these streams in events and through the createDataStream API.

We planned to keep the Peer as a Duplex by exposing the methods of a default DataChannelWrapper. So a lite version would just need to change the DataChannel implementation.

ie)

var peer = new Peer()
var dc = peer.createDataChannel()
dc.write('data') // duplex, ordinary API

peer.write('data) // peer exposes the stream API of the default datachannel
Peer.DataChannelWrapper = MyLiteDataChannel // Replace the DataChannelWrapper during build or whatever

var peer = new Peer()
var dc = peer.createDataChannel()
dc.send('data') // not a duplex, different non-stream API

peer.write // undefined, no stream API to expose

t-mullen avatar Jun 12 '18 23:06 t-mullen

This means any API the DataChannelWrapper has, the Peer also has as it proxies a default instance of it.

t-mullen avatar Jun 12 '18 23:06 t-mullen

Thanks, makes a lot of sense now. In this case I agree it might be a good idea to wait.

nazar-pc avatar Jun 12 '18 23:06 nazar-pc

Is there any news on this?

There is also a stream API in the browsers now: https://streams.spec.whatwg.org Don't know if that would help, though.

nikeee avatar Mar 27 '20 05:03 nikeee

I just opened an issue in the WebTorrent project for us to consider migrating from readable-stream to streamx. It has many potential benefits including fully removing readable-stream and buffer from the dependency tree.

If we do this, we get the following benefits:

  • remove readable-stream and buffer from the dependency tree, reducing the bundle size overhead from streaming support down to a nearly unnoticeable 2806 bytes (gzipped).
  • better streaming performance for data channel
  • preserve the same Node.js stream API that simple-peer has had since day 1, so no need for users to port their code
  • keep streaming support (.pipe(), etc.) APIs with no need to invent our own solution
  • no need to split the codebase into two parts, one with Node.js streams support and one without (as we previously considered doing)

Take a look at the webtorrent issue for more info: https://github.com/webtorrent/webtorrent/issues/1971

feross avatar Nov 24 '20 23:11 feross

Hi, not directly related but starting from @nazar-pc's work I made a fork of simple-peer which has no external dependencies at all: mitschabaude/simple-peer-light

It's on npm as well: npm install simple-peer-light. Only works in the browser and does not support the node stream API.

The motivation was to be able to use simple-peer in a Snowpack app, which relies on rollup which doesn't manage to bundle readable-stream (see https://github.com/nodejs/readable-stream/issues/348). As a nice side-effect I got an 80% size reduction for this lib.

I post this just in case it helps someone like myself. Obviously, in this repo you want to support the stream API and @feross knows the way to go.

Thank you all for this excellent library ❤️

mitschabaude avatar Jan 05 '21 10:01 mitschabaude

@mitschabaude This is excellent. I've just thought about the same, but you did it already :). Big up

bmz1 avatar Jan 27 '21 10:01 bmz1

@mitschabaude thank you sooo much... i just wasted a whole day trying to figure out how to use simple-peer with snowpack

dj-nitehawk avatar Feb 01 '21 13:02 dj-nitehawk

@mitschabaude Thanks for trying this out in the fork. I've used it in my matchmaking library and am now able to use it in Snowpack and Rollup. The much slimmer bundle size is a nice bonus too.

dmotz avatar Mar 20 '21 19:03 dmotz

this repo is unfortunately kinda dead, I've made PRs to it to replace buffer with uint8 and readable with streamx, but they aren't approved yet

for the meantime i made a package @thaunknown/simple-peer which preserves 100% of the functionality of simple peer with duplex streams etc, while dropping readable and buffer, which means the package is tiny, one major difference is that the output is uint8 not buffer so you can't run toString() on the outputs, rather you'd need some utility like arr2text from uint8-util to do it

ThaUnknown avatar May 31 '23 09:05 ThaUnknown