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

http: Check cancel flag more often #13095

Merged
merged 1 commit into from
Jul 5, 2020

Conversation

unknownbrackets
Copy link
Collaborator

Would be better to use non-blocking sockets, but this code is working, so not planning to rewrite it right now.

This just makes us run select() on sockets before reading/writing to them. Before, we would try to write, and only timeout if we got data slowly. In a situation where the user loses signal, recv() will just block. In other places, we were using a timeout - but only checking at the end of the entire timeout.

Now, we check the cancel flag in small chunks of 0.25s waits. So if the user loses connectivity, it won't just wait and wait.

Actual behavior with a good connection should not be impacted.

-[Unknown]

Would be better to use non-blocking sockets, but this code is working,
so not planning to rewrite it right now.
@hrydgard hrydgard added this to the v1.10.2 milestone Jul 5, 2020
@hrydgard
Copy link
Owner

hrydgard commented Jul 5, 2020

Thanks!

Ideally we should probably switch to some trusted crossplatform all in one solution for http downloads, or use platform specific stuff like the Android Download Manager. But yeah, not now. And I don't know what that crossplatform solution would be...

I'm gonna test this on a few platforms, and then probably merge in to 1.10.2.

@hrydgard
Copy link
Owner

hrydgard commented Jul 5, 2020

Somehow this seems to greatly improve download speed on both Windows and Android? Can't complain :)

Will try on Mac too and a couple of old Android devices, then merge if all goes well.

@hrydgard hrydgard merged commit f2ca7b7 into hrydgard:master Jul 5, 2020
@unknownbrackets
Copy link
Collaborator Author

Interesting. It seemed faster to me too but I wasn't sure. It should process the buffer more efficiently but greatly is a bit unexpected.

-[Unknown]

@unknownbrackets unknownbrackets deleted the downloads branch July 5, 2020 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants