-
Notifications
You must be signed in to change notification settings - Fork 303
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
Extract all assessment resources at once. #4868
Extract all assessment resources at once. #4868
Conversation
@MCGallaspy can you test this branch out and see if it's fast enough for Windows? |
|
||
zip_item_fileobj = zf.open(item) | ||
shutil.move(temp_assessment_dir, assessment_dest_dir) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Didn't profile, but it's down to about 10 mins. However I still think there's an easy improvement -- see my comment. |
Rather than looping over them in python code then extracting them one by one.
d52afba
to
c6af3c0
Compare
@MCGallaspy can you re-pull this branch and redownload
It still takes 15 seconds for the whole process. Maybe it'll be different for Windows. |
I don't expect it to be different because I'm on Windows, but because I have a conventional hard drive vs an SSD. Is that a bad expectation? |
Cumulative time before the latest change:
And total time:
Benchmarking the latest changes now. |
Do you have KA Lite in a separate drive vs. your Program Files? Might explain why this takes so long. |
Cumulative time for latest changes:
Total time for latest changes:
ConclusionStill prefer this branch, even though it took longer. The difference is because of Will try one more benchmark to see if it really was an anomaly. |
Nope, not anomalous! The earlier fix was faster on my machine.
My ka-lite directory and the content pack ( I'll happily merge this, but I'll wait for your reply. |
I mentioned this because it might be a difference in the copying algorithm. If you look at the shutil.move operation, it mentions that if the files are in the same filesystem, it does an Anyway, |
Btw, I tried out using |
Extract all assessment resources at once.
Rather than looping over them in python code then extracting them one by one.
Before: #4863 (comment)
After: #4863 (comment)
15 seconds vs. 26 seconds.
Might also solve the corrupt filenames getting served, as seen by @MCGallaspy.