Skip to content
This repository has been archived by the owner on Jan 13, 2024. It is now read-only.

compress files in virtual file system (#885) #1115

Merged
merged 1 commit into from
May 1, 2021

Conversation

erossignon
Copy link
Contributor

@erossignon erossignon commented Mar 31, 2021

Fixes #885

@hipstersmoothie
Copy link
Contributor

Would be cool to see some before and after data on this

@erossignon
Copy link
Contributor Author

erossignon commented Mar 31, 2021

This PR enables compression of Virtual File System table and Zip/Brotli compression of each individual file content.
This allows a gain of up to 60% of the embedded file system.

node.exe size  = 71917203

Virtual File System size: 
No compression =  175703
        Δ GZip =  -100540 ( -57 %)
      Δ Brotli =  -109057 ( -62 %)

@erossignon erossignon force-pushed the Feature/CompressFileSystem branch 2 times, most recently from 68b9521 to 5914e56 Compare March 31, 2021 19:22
@erossignon erossignon marked this pull request as draft March 31, 2021 19:22
@jesec
Copy link
Contributor

jesec commented Apr 1, 2021

If possible, IMO we can try to make it possible to compress the entire binary, not only the virtual file system.

@robertsLando
Copy link
Contributor

@erossignon Nice work here! BTW I have linked the pr that was asking for zip executables, what do you think about them? I know https://www.npmjs.com/package/caxa works in this way for example

@erossignon
Copy link
Contributor Author

If possible, IMO we can try to make it possible to compress the entire binary, not only the virtual file system.

Yes, The executable that we use from pkg_fetch is far too big, It should be compiled with minimum ICU to significantly reduce its size. then we could had some options in pkg to reference or add the ICU as a external module ( optional)

I have no idea how to compress the entire executable further... if you have a idea, let me know.

@erossignon erossignon force-pushed the Feature/CompressFileSystem branch 9 times, most recently from 9eb4d2a to 6560ded Compare April 2, 2021 21:08
@erossignon erossignon marked this pull request as ready for review April 3, 2021 07:06
@erossignon erossignon requested a review from robertsLando April 3, 2021 07:07
Copy link
Contributor

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

cc @leerob @jesec

@erossignon erossignon force-pushed the Feature/CompressFileSystem branch 5 times, most recently from 1078576 to d8021e8 Compare April 8, 2021 19:38
@erossignon
Copy link
Contributor Author

@leerob @jesec , do you have any objection that would prevent merging ?

@leerob
Copy link
Member

leerob commented Apr 9, 2021

I'd like to wait until we cut a release of pkg that's fetching from pkg-fetch properly.

@erossignon
Copy link
Contributor Author

I'd like to wait until we cut a release of pkg that's fetching from pkg-fetch properly.

OK, I'll keep rebasing and keep the code up to date until new version of pkg-fetch is fully integrated

@erossignon erossignon force-pushed the Feature/CompressFileSystem branch from d8021e8 to 751d24f Compare April 11, 2021 18:14
@erossignon erossignon force-pushed the Feature/CompressFileSystem branch from 751d24f to d217bbd Compare April 22, 2021 10:32
@Hypfer
Copy link
Contributor

Hypfer commented Apr 22, 2021

I have no idea how to compress the entire executable further... if you have a idea, let me know.

UPX might be an option

See:
#50 (comment)

Relevant quotes

https://sourceforge.net/p/upx/discussion/6806/thread/79e2a6b8/

As it is now, the readPrelude function patched into the nodejs binary will open /proc/self/exe and try reading the overlay from the offset replaced into the binary at packaging time. However since the binary found at /proc/self/exe is the compressed version, all those offsets are of course just wrong and even if they weren't they would still read compressed data.

To get pkg to work with upx, one approach could be to update the readPrelude function so that it uses relative offsets from the end of the file and extend the pkg process so that it first bakes in the commandline options, then packs that binary and finally appends the rest of the code.

@erossignon erossignon force-pushed the Feature/CompressFileSystem branch from d217bbd to 5271379 Compare April 23, 2021 11:44
@erossignon erossignon force-pushed the Feature/CompressFileSystem branch 3 times, most recently from 9bd7a83 to edc8c5e Compare April 24, 2021 20:38
@erossignon
Copy link
Contributor Author

@leerob can we incorporated to a 5.1 release ?

@leerob
Copy link
Member

leerob commented Apr 27, 2021

5.1 is out, but we can get this in 5.2! 😁

@alitas
Copy link

alitas commented Apr 29, 2021

On an unrelated note, there seems to be a typo on a few lines in the commits, both in code and documentation, using Brolti instead of Brotli. It might cause some confusion among users.

@robertsLando
Copy link
Contributor

@erossignon Could you fix the typos?

@erossignon erossignon force-pushed the Feature/CompressFileSystem branch from edc8c5e to bb1821b Compare April 30, 2021 12:32
@erossignon
Copy link
Contributor Author

let's go

@Hypfer
Copy link
Contributor

Hypfer commented Apr 30, 2021

When testing this + upx, my webserver spontaneously started to deliver compressed assets. It did work fine initially and only stopped working after a few minutes of runtime
image

This may or may not be a side-effect of the upx compression though, although considering that that never touched the assets and that the server still serves files via http just fine, I'd say that that's unlikely

@robertsLando
Copy link
Contributor

@jesec @leerob Could we merge this?

@erossignon
Copy link
Contributor Author

erossignon commented May 1, 2021

@Hypfer

It did work fine initially and only stopped working after a few minutes of runtime

This could mean that the size of the executable could change at runtime somehow? Strange! (your technique for relative offset, reads the executable size each time to find the start of the prelude by subtracting the payload size. )

This could also mean that one asset is read by chunks randomly (direct file access, set position then read ) rather than as a whole.

This behavior could be studied in a subsequent Pull Request, and should not prevent this PR to merged to main branch. At the end the compress options is not on by default.

@jesec jesec merged commit ea7d72b into vercel:master May 1, 2021
jesec added a commit that referenced this pull request May 31, 2021
…tem (#1115)"

This reverts commit 82ce625.
This reverts commit ea23196.
This reverts commit ea7d72b.

Bug: #1191, #1192
jesec added a commit that referenced this pull request May 31, 2021
This reverts commit 82ce625.
This reverts commit ea23196.
This reverts commit ea7d72b.

Bug: #1191, #1192
jesec added a commit that referenced this pull request May 31, 2021
This reverts commit 82ce625.
This reverts commit ea23196.
This reverts commit ea7d72b.

Bug: #1191, #1192
erossignon added a commit to node-opcua/pkg that referenced this pull request May 31, 2021
jesec pushed a commit that referenced this pull request Jun 3, 2021
This reverts commit 52ddf23.

v2:

- #1191
- only compress VFS if --compress is set (#1192)
erossignon added a commit to node-opcua/pkg that referenced this pull request Jun 3, 2021
erossignon added a commit to node-opcua/pkg that referenced this pull request Jun 5, 2021
erossignon added a commit to node-opcua/pkg that referenced this pull request Jun 7, 2021
erossignon added a commit to node-opcua/pkg that referenced this pull request Jun 8, 2021
erossignon added a commit to node-opcua/pkg that referenced this pull request Jun 8, 2021
jesec pushed a commit that referenced this pull request Jun 8, 2021
)

v2:

- fix issue #1191 
- only compress VFS if --compress is set (#1192)
- improve random access with the compressed virtual file system
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Zip executables ?
7 participants