Skip to content

Refactor zip filesystem#1777

Open
Schamper wants to merge 3 commits into
mainfrom
refactor-zipfs
Open

Refactor zip filesystem#1777
Schamper wants to merge 3 commits into
mainfrom
refactor-zipfs

Conversation

@Schamper

@Schamper Schamper commented Jun 4, 2026

Copy link
Copy Markdown
Member

As a follow up to #1772, this PR aims to simplify and speed up the zip filesystem implementation a bit by getting rid of the dependency of the virtual filesystem, as well as hopefully dealing with some more edge cases.

I took some inspiration from how zipfile.ZipPath works for this.

Correctly dealing with all the paths and variations is a bit tricky, so I'm sure there's room for improvement. This is also a bit of a test run before I commit to the tar variant, since that has a lot of loaders that depend on the implementation details of the current tar filesystem.

Fingers crossed that Codspeed agrees this is faster.

@codspeed-hq

codspeed-hq Bot commented Jun 4, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 11.54%

⚡ 1 improved benchmark
❌ 8 regressed benchmarks
✅ 23 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
test_benchmark_zip_filesystem[absolute-base-dir] 90.6 ms 110.1 ms -17.77%
test_benchmark_zip_filesystem[absolute-base] 90.6 ms 109.9 ms -17.55%
test_benchmark_zip_filesystem[base] 90.6 ms 109.1 ms -16.95%
test_benchmark_zip_filesystem[relative] 91.2 ms 108 ms -15.6%
test_benchmark_zip_filesystem[relative-dir] 91.1 ms 107.8 ms -15.46%
test_benchmark_zip_filesystem[absolute] 91 ms 104.5 ms -12.96%
test_benchmark_zip_filesystem[simple] 90.3 ms 103.4 ms -12.62%
test_benchmark_zip_filesystem[virtual-dir] 90.4 ms 103.4 ms -12.55%
test_benchmark[_data/loaders/uac/uac-2e44ea6da71d-linux-20250717143106.zip-ZipLoader] 17.8 ms 14.4 ms +24.06%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing refactor-zipfs (f034d34) with main (6e4e1fc)

Open in CodSpeed

@codecov

codecov Bot commented Jun 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.26027% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.30%. Comparing base (6e4e1fc) to head (f034d34).
⚠️ Report is 24 commits behind head on main.

Files with missing lines Patch % Lines
dissect/target/filesystems/zip.py 97.10% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1777      +/-   ##
==========================================
+ Coverage   81.20%   81.30%   +0.10%     
==========================================
  Files         405      405              
  Lines       35879    35900      +21     
==========================================
+ Hits        29134    29190      +56     
+ Misses       6745     6710      -35     
Flag Coverage Δ
unittests 81.30% <97.26%> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

1 participant