Skip to content

Perf parse params decode text - #109

Merged
Uzlopak merged 1 commit into
masterfrom
perf-parseParams-decodeText
Oct 10, 2023
Merged

Uzlopak merged 1 commit into
masterfrom
perf-parseParams-decodeText

Conversation

@Uzlopak

@Uzlopak Uzlopak commented Jan 8, 2023

Copy link
Copy Markdown
Contributor

Based on the work of mscdex regarding the decodeText. Pretty smart how he did it.

Checklist

@Uzlopak
Uzlopak requested review from Eomm and kibertoad January 8, 2023 02:10

@Eomm Eomm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Based on the work of mscdex regarding the decodeText. Pretty smart how he did it.

Now sure to understand: did we charry-pick some work from the original busboy repo? - In case I would add credits

@Eomm Eomm mentioned this pull request Jan 8, 2023
4 tasks
@kibertoad

Copy link
Copy Markdown
Member

@Uzlopak Can you resolve the conflicts?

@kibertoad

Copy link
Copy Markdown
Member

@Uzlopak what are the benchmarks agains the master?

@Uzlopak

Uzlopak commented Sep 28, 2023

Copy link
Copy Markdown
Contributor Author

@kibertoad
PTAL

@kibertoad

Copy link
Copy Markdown
Member

@Uzlopak can you run benchmark to see the perf difference?

@Uzlopak

Uzlopak commented Sep 28, 2023

Copy link
Copy Markdown
Contributor Author

before:
aras@aras-Lenovo-Legion-5-17ARH05H:~/workspace/busboy$ node bench/parse-params.js
video/ogg x 2,298,591 ops/sec ±0.80% (92 runs sampled)
'text/plain; filename*=utf-8''%c2%a3%20and%20%e2%82%ac%20rates' x 257,143 ops/sec ±0.29% (96 runs sample

after:
aras@aras-Lenovo-Legion-5-17ARH05H:~/workspace/busboy$ node bench/parse-params.js
video/ogg x 2,579,688 ops/sec ±0.31% (90 runs sampled)
'text/plain; filename*=utf-8''%c2%a3%20and%20%e2%82%ac%20rates' x 330,596 ops/sec ±0.26% (97 runs sampled)

Comment thread bench/busboy-form-bench-latin1.js Outdated
Comment thread package.json Outdated
@Uzlopak

Uzlopak commented Sep 28, 2023

Copy link
Copy Markdown
Contributor Author

@kibertoad
I dont know... tbh photofinish is a little bit complicated. I maybe rewrite the new benchmark in tinybench. Or can you rewrite that benchmark from benchmark to photofinish?

@kibertoad

Copy link
Copy Markdown
Member

@Uzlopak We can rewrite to tinybench, photofinish main strength is that it allows running easily across Node versions, which is an overkill here.

@Uzlopak
Uzlopak force-pushed the perf-parseParams-decodeText branch from 336318d to 5dc9096 Compare September 28, 2023 21:54
@Uzlopak

Uzlopak commented Oct 6, 2023

Copy link
Copy Markdown
Contributor Author

@kibertoad
can we just merge? I have no motivation to rewrite the benchmarks. Maybe create an issue to rewrite the benchmarks and slab a 'good-first-issue' label to it.

@Uzlopak
Uzlopak merged commit a616201 into master Oct 10, 2023
@Uzlopak
Uzlopak deleted the perf-parseParams-decodeText branch October 10, 2023 09:19
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Sep 1, 2026
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