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

Improve performance in searchstream #36

Merged
merged 3 commits into from
Nov 29, 2021
Merged

Improve performance in searchstream #36

merged 3 commits into from
Nov 29, 2021

Conversation

Uzlopak
Copy link
Contributor

@Uzlopak Uzlopak commented Nov 29, 2021

Unit tests are green, so I think the change provides an equivalent functionality.

Screenshot from 2021-11-29 19-10-02

Benchmark:

> @fastify/[email protected] bench:dicer
> node deps/dicer/bench/dicer-bench-multipart-parser.js

9090.91 mb/sec
11111.11 mb/sec
10000.00 mb/sec
11111.11 mb/sec
11111.11 mb/sec
11111.11 mb/sec
12500.00 mb/sec
12500.00 mb/sec
11111.11 mb/sec
11111.11 mb/sec

Checklist

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Nov 29, 2021

Also deoptimization traces are gone.

node --trace-deopt --trace-opt ./deps/dicer/bench/dicer-bench-multipart-parser.js

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Nov 29, 2021

I think this PR is actually good enough performance gain for this iteration, so that even @mcollina would be happy :-D as it does not only shaves some yoctoseconds.

So we improved the performance from about 3,5 GB/s to 10-12 GB/s.

@kibertoad
Copy link
Member

will take a look later tonight, need to wrap up version release at work

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@kibertoad kibertoad merged commit aedb42c into fastify:master Nov 29, 2021
@kibertoad
Copy link
Member

@Uzlopak BTW, I've published a beta version of the package and tried running fastify-multipart as well as multer v2 tests against it, and all of them are still passing, so looks like your optimizations didn't break anything :)

For the first public release I still want to wait for #26
Is there anything else that is on your "must have" list for 1.0.0?

@Uzlopak Uzlopak deleted the performance-streamsearch branch November 30, 2021 02:04
@Uzlopak
Copy link
Contributor Author

Uzlopak commented Nov 30, 2021

@kibertoad
I think we already achieved alot :). Just let us merge the last PRs and then release it :)

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.

3 participants