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

Consider switching to undici library for http requests #134

Closed
ppedziwiatr opened this issue Jan 6, 2022 · 6 comments
Closed

Consider switching to undici library for http requests #134

ppedziwiatr opened this issue Jan 6, 2022 · 6 comments

Comments

@ppedziwiatr
Copy link
Contributor

https://www.npmjs.com/package/undici

Simple benchmark:
image

image

That's 2x the difference...

@ppedziwiatr
Copy link
Contributor Author

ppedziwiatr commented Jan 6, 2022

obviously just for the node.js env.

A bit more direct comparison with Axios:
image

image

....I believe this could have a huge impact on - for example - loading pages from the GQL endpoint.

@ppedziwiatr
Copy link
Contributor Author

ppedziwiatr commented Jan 6, 2022

Yup, just like I've expected - huge performance boost while loading SmartWeave interacitons for our "loot" contract - from ~61s on default arweave.js (with Axios underneath) to ~40s on undici 🚀 20s difference...

arweave.js/axios
image
image

undici:
image
image

@rosmcmahon
Copy link
Member

rosmcmahon commented Jan 7, 2022

it does not support the browser. axios is a good all-rounder.

if you replaced axios with undici for nodejs, would you also replace it with something in the browser? are native browser fetch calls faster?

the code base is already split with a custom solution supporting nodejs crypto and webcrypto, so it's not totally unreasonable to split it further - although the cypto-drivers could probably be replaced by a single external library ? - but that's off topic

I will let someone else way in with opinion here, but if we decide to go that way would you be willing to do a PR for it?

@ppedziwiatr
Copy link
Contributor Author

I believe the best solution would be to prepare an isomorphic-fetch, with undici as a library used by node (sth. similar to https://www.npmjs.com/package/isomorphic-fetch).
We will prepare sth. similar for our contracts sdk and will let you know if it works as expected. If it will, you could consider using it in arweave-js.

@ppedziwiatr
Copy link
Contributor Author

maybe it could also speed up a bit #128

@ppedziwiatr
Copy link
Contributor Author

we've created https://github.com/redstone-finance/redstone-isomorphic (which has isomorphic version of fetch - uses undici for node and native fetch for web).
But I assume you're currently not interested in using this (and we've already created a wrapper for arweave-js in our SDK), so I'm closing this issue.

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

No branches or pull requests

2 participants