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

exposing the content-type parser? #1891

Closed
KhafraDev opened this issue Jan 27, 2023 · 7 comments · Fixed by #1895
Closed

exposing the content-type parser? #1891

KhafraDev opened this issue Jan 27, 2023 · 7 comments · Fixed by #1895

Comments

@KhafraDev
Copy link
Member

KhafraDev commented Jan 27, 2023

Since undici's parser consistently outperforms node's, should we consider exposing it? It already passes the entirety of the mimesniff tests.

Originally posted by @KhafraDev in nodejs/performance#38 (comment)

@KhafraDev
Copy link
Member Author

... and unlike FileReader, is actually relevant to an http client. It's low maintenance and isn't affected by node's release schedule (it's much easier to update undici to a newer version than a newer node version if a bug appears in either parser).

@metcoder95
Copy link
Member

It is also important to highlight that some adjustments can be made to Node compared to Undici's implementation. As, e.g., the collectASequenceOfCodePoints can be totally optimized only to be used by the MIME class, outcoming even better results.
This can also be achieved at Undici if scoping the same function only to MIME class, but yeah worth it to draw that out 🙂

@KhafraDev
Copy link
Member Author

KhafraDev commented Jan 28, 2023

collectASequenceOfCodePoints could probably be replaced with indexOf and slice, unsure if it would be faster

@mcollina
Copy link
Member

I think replacing node mime parser with undici's is a decision better taken in the Node.js repo.

@KhafraDev
Copy link
Member Author

I don't think it needs to be replaced, I'm wondering if undici should export it as an alternative to node's.

@mcollina
Copy link
Member

Definitely!!

@metcoder95
Copy link
Member

metcoder95 commented Jan 29, 2023

collectASequenceOfCodePoints could probably be replaced with indexOf and slice, unsure if it would be faster

As shared in your PR, it definitely is. Didn't make that comparison within Undici but was able to come to the same conclusion by running external benchmarks comparing indexOf, search, and the Regex#test method. So far showed similar results, but the former always taking the lead.

Big step forward exposing the util 🚀

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 a pull request may close this issue.

3 participants