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

Reuse downloader client to help lower network overhead #128

Open
Timbus opened this issue Dec 18, 2020 · 2 comments
Open

Reuse downloader client to help lower network overhead #128

Timbus opened this issue Dec 18, 2020 · 2 comments
Labels
enhancement New feature or request

Comments

@Timbus
Copy link

Timbus commented Dec 18, 2020

https://github.com/hkalexling/Mango/blob/df51406638d7266a990bd2b660e3bcc2d2d71c53/src/mangadex/downloader.cr#L142
Should probably reuse a connected client for downloads, which should reduce overhead a tiny amount.

Just hang on to a client using a lazy getter I suppose: https://crystal-lang.org/api/master/HTTP/Client.html#reusing-a-connection

@hkalexling
Copy link
Member

Thanks for the suggestion! I am not very familiar with the Crystal compiler internals and the GC but I think the performance improvement would be very minimal if any. But I guess it doesn't hurt to optimize a bit :P Are you interested in submitting a PR to the dev branch?

@hkalexling hkalexling added the enhancement New feature or request label Dec 18, 2020
@Timbus
Copy link
Author

Timbus commented Dec 19, 2020

Sure thing: #130

Also regarding performance, the problem with reconnecting each time is that you need to allocate a socket, do the TCP handshake, generate new TLS keys, etc etc.. So it's not really related to Crystal's performance (which is excellent, yes), it's mostly network related.
Avoiding this overhead is mostly for mangadex's benefit, but it does slightly benefit the client too.

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

No branches or pull requests

2 participants