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

Performance regression when opening lots of zip archives #311

Open
LGriffioen opened this issue Feb 2, 2024 · 2 comments · May be fixed by #337
Open

Performance regression when opening lots of zip archives #311

LGriffioen opened this issue Feb 2, 2024 · 2 comments · May be fixed by #337

Comments

@LGriffioen
Copy link
Contributor

LGriffioen commented Feb 2, 2024

Summary

We have a ton of uncompressed zip archives that are read and written using ZipFoundation, and users can load all of them at once in a library view. I have 8,000+ items and before the commit linked below, these could all load in 1.2s, but after it takes over 12 seconds in debug. In release mode it was 1s before and 6s after.

Both tests were done on an M1 Max machine

Steps to Reproduce

  1. create thousands of uncompressed zips, can contain multiple files
  2. load them all in a loop

Expected Results

should be very fast, even in debug

Actual Results

it's gotten ~6x slower in release, and 10x slower in debug

Regression & Version

introduced in b3ca1e6

Related Link

@weichsel
Copy link
Owner

weichsel commented Feb 3, 2024

Hi Luke,

Thanks for investigating. I just skimmed over the changes and on a first glance, this change here could be the source of the performance degradation: b3ca1e6#diff-d829f12a7b0fa38c616a3397c9ea1464287b0d0097dee344876fff88b4b28e39L316

Currently don't have the time to optimize this but if you can provide a PR, I can have a look.

@LGriffioen
Copy link
Contributor Author

you're correct! adding that one check back in fixes the issue. the question is why was it removed, and what might break by restoring it?

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

Successfully merging a pull request may close this issue.

2 participants