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

WIP: Improve audit time #545

Draft
wants to merge 36 commits into
base: main
Choose a base branch
from
Draft

WIP: Improve audit time #545

wants to merge 36 commits into from

Conversation

oraluben
Copy link
Contributor

@oraluben oraluben commented Feb 19, 2025

Motivation: We're using auditwheel to repair our internal PyTorch build, the overall time is ~10m. We're attempting to reduce the redundant operations that we've found, e.g. unzips.

This PR is not finalized yet but reviews are welcome!

  • add log for time-consuming parts
  • do not re-unzip wheel if possible
  • multiprocess grafting libs
  • set default compress level to 4
  • tests
  • cleanup

@oraluben oraluben force-pushed the fast branch 2 times, most recently from ed48220 to b449541 Compare February 19, 2025 03:23
@oraluben
Copy link
Contributor Author

oraluben commented Feb 19, 2025

Based on a wheel of pytorch, I test the speed and size of different compress level of zip.

There's some outliner in extract time, but I haven't double check them yet, let's just ignore them.

Based on size, 0->1 reduced over 50% size and 3->4 reduced >1%, all other levels have a <1% contribution. It should be reasonable to set default level (from 6) to 4, and adding another cli argument to specify compress level (to 0 or 9 if user know what they're doing) also makes sense to me.

so files only:

level size (% of store) zip time (s) extract time (s)
0 1511382547(100.0%) 4.21 8.35
1 791288879(52.36%) 35.41 12.75
2 783875121(51.86%) 40.83 13.55
3 777160594(51.42%) 48.52 13.5
4 758317484(50.17%) 51.45 13.64
5 750978688(49.69%) 62.49 12.94
6 746963476(49.42%) 78.61 12.77
7 746170783(49.37%) 85.38 12.7
8 745399675(49.32%) 105.02 12.5
9 745330440(49.31%) 117.64 12.47

all files:

level size (% of store) zip time (s) extract time (s)
0 2383315752(100.0%) 6.85 13.74
1 1353275864(56.78%) 58.63 20.86
2 1343694522(56.38%) 65.84 22.95
3 1335117720(56.02%) 79.87 46.13
4 1305739693(54.79%) 85.3 21.8
5 1295768731(54.37%) 102.86 22.34
6 1290671655(54.15%) 125.37 21.24
7 1289634091(54.11%) 132.69 20.91
8 1288638871(54.07%) 158.58 20.74
9 1288528808(54.06%) 176.87 21.66

Copy link

codecov bot commented Feb 19, 2025

Codecov Report

Attention: Patch coverage is 86.48649% with 10 lines in your changes missing coverage. Please review.

Project coverage is 91.48%. Comparing base (b01e842) to head (aefee5e).

Files with missing lines Patch % Lines
src/auditwheel/wheeltools.py 64.28% 4 Missing and 6 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #545      +/-   ##
==========================================
- Coverage   92.07%   91.48%   -0.60%     
==========================================
  Files          22       22              
  Lines        1553     1597      +44     
  Branches      285      293       +8     
==========================================
+ Hits         1430     1461      +31     
- Misses         74       81       +7     
- Partials       49       55       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@oraluben
Copy link
Contributor Author

oraluben commented Feb 20, 2025

With the demo in PR, the time has been reduced from 3m36.515s to 2m44.353s. Concurrent grafting seems not significent, I'll see the result on our real workload later.

Only change compress level to 4: 2m53.029s

@oraluben oraluben marked this pull request as ready for review February 20, 2025 12:03
@oraluben
Copy link
Contributor Author

Numbers:
10m59.651s (6.2.0)
10m27.363s (current version + level=6)
8m54.623s (current version)
5m14.166s (current version + level=0)

The repair part still seem potential to be optimized.

audit_orig.log
audit_fast.log

@mayeut
Copy link
Member

mayeut commented Feb 23, 2025

@oraluben, thanks for digging into this.
I'd keep the default level for compression to 6. While the less than 1% (around .5/.6%) improvement does not seem like a big one, once you scale that to "applies to all PyPI" downloads / storage, the .5/.6% improvement applies for all downstream usage. The .1% improvement between 6 & 9 is probably not worth it (although one could re-use the argument I just used to say that it is).
Given the extract time is roughly the same for every level - except 0 obviously -, my feeling is that it's better to spend a bit more time get better compression for each of the package published (around 10,000 manylinux packages) on PyPI rather than to increase even by .5% the storage/bandwidth spent retrieving those packages (42,362,259 downloads of manylinux packages last Friday).

@mayeut
Copy link
Member

mayeut commented Feb 23, 2025

For the " do not re-unzip wheel if possible", I'd probably move unzipping at the main_repair/main_show level not to introduce complexity with caching.

@oraluben oraluben marked this pull request as draft February 23, 2025 12:26
@oraluben oraluben mentioned this pull request Mar 7, 2025
4 tasks
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 this pull request may close these issues.

2 participants