Skip to content

Comments

Disable lua compilation based on file size#263

Merged
squeek502 merged 1 commit intoluvit:masterfrom
SinisterRectus:master
Jan 25, 2020
Merged

Disable lua compilation based on file size#263
squeek502 merged 1 commit intoluvit:masterfrom
SinisterRectus:master

Conversation

@SinisterRectus
Copy link
Member

@SinisterRectus SinisterRectus commented Oct 8, 2019

Lit currently bundles compiled .lua files if their bytecode is smaller in size than the original source file. I propose that we change this.

Issue 1: The attempt at space saving is negligible or detrimental.

Using lit itself as an example of a bundled luvi 2.9.3 app on Windows, the resulting file sizes are:

No compilation: 4,123 KB
Partial compilation (current behavior): 4,144 KB
Full compilation: 4,148 KB

While there is sometimes a reduction in individual .lua file sizes, there is actually an overall increase in total executable size here. Presumably bytecode is being zipped with less efficiency than the source? Additionally, luvi contributes over 90% of the executable size here, so there isn't much room for space negotiation.

Issue 2: Lua compiled using one luvi version may be incompatible with other versions.

More specifically, if the version of luvi used to build lit is different from the version of luvi that lit uses to build another app, the bytecode may not be compatible if different luac versions are involved. See #252.


Precompiling to bytecode isn't inherently bad! We just aren't using it in the right way. From the luac man page:

The main advantages of precompiling chunks are: faster loading, protecting source code from accidental user changes, and off-line syntax checking. Precompiling does not imply faster execution because in Lua chunks are always compiled into bytecodes before being executed. luac simply allows those bytecodes to be saved in a file for later execution. Precompiled chunks are not necessarily smaller than the corresponding source. The main goal in precompiling is faster loading.

What this PR does is remove the logic for compiling based on file size, makes compilation and all-or-nothing procedure, and changes the default behavior to no compilation.

What this PR does NOT do is add the option to select whether files should be compiled from the CLI. I'm open for suggestions on how to handle that, or we can save that for a later change.

Fixes #252.

Happy Hacktoberfest

Copy link
Member

@squeek502 squeek502 left a comment

Choose a reason for hiding this comment

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

Nice, definitely think this is a worthwhile change.

@squeek502
Copy link
Member

Whoops, forgot to merge this.

@squeek502 squeek502 merged commit ecab48c into luvit:master Jan 25, 2020
@SinisterRectus
Copy link
Member Author

No worries. I wasn't sure that you had planned to anyway.

squeek502 added a commit to squeek502/lit that referenced this pull request Mar 9, 2020
….exe

If there was an older lit.exe or luvi.exe on your PATH, then running get-lit.ps1 anywhere would use those older existing lit.exe/luvi.exe instead of the newly downloaded ones. This could lead to 'cannot load incompatible bytecode' errors that should no longer be possible after luvit#263.
squeek502 added a commit that referenced this pull request Mar 9, 2020
….exe (#268)

If there was an older lit.exe or luvi.exe on your PATH, then running get-lit.ps1 anywhere would use those older existing lit.exe/luvi.exe instead of the newly downloaded ones. This could lead to 'cannot load incompatible bytecode' errors that should no longer be possible after #263.
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.

lit make lit://luvit/luvit creates unusable binary after get-lit

2 participants