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

Encoder calls Readable.push #5

Open
hildjj opened this issue Mar 11, 2014 · 8 comments
Open

Encoder calls Readable.push #5

hildjj opened this issue Mar 11, 2014 · 8 comments

Comments

@hildjj
Copy link
Owner

hildjj commented Mar 11, 2014

This causes an unneeded copy. Perhaps make BufferStream Readable instead of Writable, and copy the push semantics in? Perhaps look at patching Readable upstream to allow unbuffered access one day.

@hildjj hildjj added bug and removed bug labels Mar 11, 2014
@dignifiedquire
Copy link
Contributor

I was interested in how fast I could get cbor parsing, so I forked this library, and dropped all streaming moving to a pure sync interface to see how fast I could get things, current results on Node 7 are below

encode - node-cbor - 74 x 206 ops/sec ±5.20% (55 runs sampled)
encode - fast-cbor - 74 x 3,586 ops/sec ±33.60% (77 runs sampled)
encode - JSON.stringify - 74 x 12,200 ops/sec ±4.47% (68 runs sampled)
decode - node-cbor - 45 x 156 ops/sec ±4.78% (54 runs sampled)
decode - fast-cbor - 45 x 4,544 ops/sec ±5.17% (53 runs sampled)
decode - JSON.parse - 45 x 15,444 ops/sec ±11.54% (49 runs sampled)

The repo is here if you are interested : https://github.com/dignifiedquire/fast-cbor

@hildjj
Copy link
Owner Author

hildjj commented Nov 28, 2016

if we're going to compare against JSON.parse, and go full-sync, then we should move the code into C. One starting point is https://github.com/cabo/cn-cbor, but others are listed here: http://cbor.io/impls.html

@dignifiedquire
Copy link
Contributor

dignifiedquire commented Nov 28, 2016

@hildjj yeah I can't really do that because I have to run it in the browser, I was just curious how close JavaScript could come to something like the JSON methods

@hildjj
Copy link
Owner Author

hildjj commented Nov 28, 2016

What if you proved that it was faster than JSON if implemented in C, at least one browser vendor decided to give you a native implementation, and we made one of these versions a polyfill?

(I work at Mozilla, but not on the platform team. I'll have to give them a patch and get them interested in taking it)

@dignifiedquire
Copy link
Contributor

Fun fact, the code even though the encoder is written in hand rolled asm.js is a decent factor slower in the latest Firefox compared to Chrome which I didn't expect. But I haven't done any detailed analysis as to where exactly that come from.

What if you proved that it was faster than JSON if implemented in C, at least one browser vendor decided to give you a native implementation, and we made one of these versions a polyfill?

That would be pretty cool, I think the first thing to test would be adding a native version for node, as that is a bit easier to experiment with, than a browser patch (at least in my experience,... still trying to get servo to build :D)

@hildjj
Copy link
Owner Author

hildjj commented Nov 28, 2016

agree. Added #40 to track that explicitly. In the meantime, are you open to merging your fork back into this project for the synchronous path?

@dignifiedquire
Copy link
Contributor

Sure, I just changed so much that I wasn't sure if and how you would want to merge those changes in here.

@hildjj
Copy link
Owner Author

hildjj commented Nov 28, 2016

Your perf results are compelling. I still want to be able to use the streaming mode if possible, code size matters somewhat, and I don't want to have to fix bugs in two places. Other than that, I'm open to lots of approaches, including patches that are relatively invasive.

sake pushed a commit to ecsec/node-cbor that referenced this issue Mar 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants