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

Avoid object allocation when creating strings from the underlying buffer #33

Merged

Conversation

rklaehn
Copy link

@rklaehn rklaehn commented Oct 8, 2018

For me this makes a significant difference WRT decoding performance, since fewer short lived copies of stringsbuffers are being created.

See https://gist.github.com/rklaehn/ba52f94896788a67107e3d38fc37a498

@rklaehn
Copy link
Author

rklaehn commented Oct 8, 2018

Here are the builtin benchmarks:

master:

PID: 677
encode - node-cbor - 76 x 622 ops/sec ±0.83% (74 runs sampled)
encode - borc - 76 x 15,305 ops/sec ±3.40% (82 runs sampled)
encode - stream - borc - 76 x 5,821 ops/sec ±7.12% (74 runs sampled)
encode - JSON.stringify - 76 x 27,993 ops/sec ±7.05% (74 runs sampled)
decode - node-cbor - 47 x 1,453 ops/sec ±1.27% (82 runs sampled)
decode - borc - 47 x 14,708 ops/sec ±2.31% (79 runs sampled)
decode - JSON.parse - 47 x 49,168 ops/sec ±6.08% (65 runs sampled)

this branch:

PID: 322
encode - node-cbor - 76 x 581 ops/sec ±1.28% (76 runs sampled)
encode - borc - 76 x 15,611 ops/sec ±3.07% (84 runs sampled)
encode - stream - borc - 76 x 6,051 ops/sec ±3.80% (79 runs sampled)
encode - JSON.stringify - 76 x 27,777 ops/sec ±7.35% (73 runs sampled)
decode - node-cbor - 47 x 1,509 ops/sec ±1.40% (82 runs sampled)
decode - borc - 47 x 31,221 ops/sec ±3.93% (82 runs sampled)
decode - JSON.parse - 47 x 50,254 ops/sec ±6.10% (65 runs sampled)

@rklaehn
Copy link
Author

rklaehn commented Oct 10, 2018

Not sure what is wrong with the CI, but The command "yarn" failed and exited with 1 during . seems unrelated to my change.

@dignifiedquire
Copy link
Owner

yeah CI is broken independently

@dignifiedquire dignifiedquire merged commit e609298 into dignifiedquire:master Oct 18, 2018
@dignifiedquire
Copy link
Owner

Thanks, and sorry for the delay

@rklaehn
Copy link
Author

rklaehn commented Oct 18, 2018

No problem. Thanks for merging this. I want to move from JSON to CBOR for some use case, and CBOR not being much slower to parse will definitely help with that tradeoff...

@dignifiedquire
Copy link
Owner

dignifiedquire commented Oct 18, 2018 via email

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

Successfully merging this pull request may close these issues.

None yet

2 participants