larsiusprime / polymod

Atomic modding framework for Haxe
MIT License
159 stars 60 forks source link

Added support for compressed zip files in ZipFileSystem #142

Closed UncertainProd closed 1 year ago

UncertainProd commented 1 year ago

This PR adds support for reading compressed zip files as well to the ZipFileSystem classes (shout out to haxe.zip.InflateImpl, which I didn't realize was already cross-platform lol)

EliteMasterEric commented 1 year ago

Does reading individual files from a compressed ZIP require loading the entire ZIP into memory?

UncertainProd commented 1 year ago

On sys targets it doesn't seem to need it, as it just seeks to that point in the zip file and reads the bytes. But since html5 doesn't have a way of seeking through files, it ends up storing the entire zip in memory for the web. Although, I'm not sure how to inspect how much of the zip is loaded into memory, do you know of any way?

EliteMasterEric commented 1 year ago

I was thinking you'd assign the program a very large ZIP then test it like that, but since it's using seek on Sys platform it probably does work correctly

On Fri, Nov 25, 2022, 01:29 UncertainProd @.***> wrote:

On sys targets it doesn't seem to need it, as it just seeks to that point in the zip file and reads the bytes. But since html5 doesn't have a way of seeking through files, it ends up storing the entire zip in memory for the web. Although, I'm not sure how to inspect how much of the zip is loaded into memory, do you know of any way?

— Reply to this email directly, view it on GitHub https://github.com/larsiusprime/polymod/pull/142#issuecomment-1327069645, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABDLVRXOI4UX7PFFASGBPW3WKBMDNANCNFSM6AAAAAASKPTXDM . You are receiving this because you commented.Message ID: @.***>

UncertainProd commented 1 year ago

Looks like it doesn't load the entire zip, I put some large images along with some small ones into one of the zips and only tried to load the small one and it didn't seem be affected by how long it would have taken to load the large image

EliteMasterEric commented 1 year ago

Makes sense. Were you looking at duration or memory usage there?

On Sat, Nov 26, 2022, 15:50 UncertainProd @.***> wrote:

Looks like it doesn't load the entire zip, I put some large images along with some small ones into one of the zips and only tried to load the small one and it didn't seem be affected by how long it would have taken to load the large image

— Reply to this email directly, view it on GitHub https://github.com/larsiusprime/polymod/pull/142#issuecomment-1328112047, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABDLVRTEIXF6LQ5NX7ZBNILWKJZYNANCNFSM6AAAAAASKPTXDM . You are receiving this because you commented.Message ID: @.***>

UncertainProd commented 1 year ago

Duration

EliteMasterEric commented 1 year ago

I merged this after some cleanup but it appears Github doesn't understand that, guess I'll just close this.

Good work :+1: