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

Re-investigate download speed regression with 0.16.0-betaX #736

Open
Tracked by #750
wjt opened this issue Aug 8, 2023 · 4 comments
Open
Tracked by #750

Re-investigate download speed regression with 0.16.0-betaX #736

wjt opened this issue Aug 8, 2023 · 4 comments

Comments

@wjt
Copy link
Member

wjt commented Aug 8, 2023

Originally posted by @vanessa-chang in #729 (comment)
Test on a chromebook, the download time takes longer compared to the production page.
chromebook

Quoth @dbnicholson:

It would be nice to verify that downloads are actually fixed, but I think this is for the best and any further regressions should probably be pursued upstream.

As I recall the original regression was more like an order of magnitude, so downloads "only" taking twice as long is still an improvement.

@dbnicholson wrote a script to benchmark different Kolibri versions:

https://gist.github.com/dbnicholson/c9ffc907546d8b866ad6210f6442a2e1

It would be interesting to rerun this with 0.16.0-betaX

@wjt wjt added this to the Manatee milestone Aug 8, 2023
@dbnicholson
Copy link
Member

I repurposed my script from learningequality/kolibri#10848 to do a simple single channel comparison:

Min 0.15 time 7.28 seconds
Min 0.16 time 8.46 seconds

This is with 0.16.0-beta1. It's still a 16% regression, but not the order of magnitude I was seeing before. I'm not sure what to make of @vanessa-chang's results since those are more like a 50% increase. Probably have to dig in further.

@jofilizola jofilizola mentioned this issue Aug 16, 2023
13 tasks
@dbnicholson
Copy link
Member

Perusing some logs flying by on Android I still see all the partial range requests (with 206 responses). I can't really make sense of the FileDownload class anymore, but it's certainly never going to be as fast as a single request for the whole file. What I don't get is that it seems like the incremental download case is already supported out of the box using requests' streaming support. Write the content to the file as it's received and update the progress according to the written size vs the size in the Content-Length header. I must be missing something, though.

What I think would be nice but would certainly require upstream agreement would be specifying whether a full or incremental download is desired. In the collection downloader I almost certainly think we just want to get the files as fast as possible.

@jofilizola
Copy link

@dbnicholson is this something that will go back to general backlog (no sprint)? Are we good with how things are for now?

@dbnicholson
Copy link
Member

I don't think we have the resources to keep digging into this right now, so yes I think it will have to go in the backlog. My hope is to fix #592 this sprint and just do less downloading to get out of the welcome screen faster.

@dbnicholson dbnicholson removed this from the Manatee milestone Aug 22, 2023
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

3 participants