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

Add qjsc -r flag to output raw bytecode #460

Merged
merged 10 commits into from
Jul 4, 2024

Conversation

TooTallNate
Copy link
Contributor

I want to allow users to ship a raw .jsc bytecode file for faster bootup times.

@saghul
Copy link
Contributor

saghul commented Jul 3, 2024

How will you handle handle bytecode incompatibility?

Edit, hit enter too fast. Bytecode generated by a given version of QJSC need not be compatible with previous or further versions of the engine. There is a BC version we bump when we make changes to the way bytecode is (de)serialized.

I support the approach, but I worry people are going to run into problems because this incompatibility is not obvious...

@TooTallNate
Copy link
Contributor Author

How will you handle handle bytecode incompatibility?

QuickJS already throws a syntax error, so on the technical level I'm not sure what else would need to be done: https://github.com/quickjs-ng/quickjs/blob/07fa1cbc4a8938085a716051c0f14a3453a885fb/quickjs.c#L35116-L35120

Socially, I plan on shipping an emscripten-compiled version of qjsc with my runtime, so the user code would always be compiled against the same quickjs version as the runtime itself. I suppose implementors might want to come up with their own approaches.

@saghul
Copy link
Contributor

saghul commented Jul 3, 2024

Sounds reasonable. We can also look into the syntax error message and make it more helpful if need be as well.

qjsc.c Outdated
@@ -450,25 +462,29 @@ int main(int argc, char **argv)
ctx = JS_NewContext(rt);

/* loader for ES6 modules */
JS_SetModuleLoaderFunc(rt, NULL, jsc_module_loader, NULL);
OutputTypeEnum *output_type_ptr = (OutputTypeEnum *)js_malloc(ctx, sizeof(OutputTypeEnum));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't you make the output type a static variable at the top of the file and simplify this? There is already precedent with things like outfile and strip and then you don't need to shove it in there

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@saghul saghul merged commit aa5e4d2 into quickjs-ng:master Jul 4, 2024
50 checks passed
@chqrlie
Copy link
Collaborator

chqrlie commented Jul 4, 2024

There is a problem: the output file must be open in binary mode to output the raw bytecode:

    if (output_type == OUTPUT_RAW)
        fo = fopen(cfilename, "wb");
    else
        fo = fopen(cfilename, "w");

This is necessary for legacy architectures with end-of-line translation and other obscure text mode conversion schemes.
I would have also preferred -b (--bytecode) over -r for this option.

@TooTallNate TooTallNate deleted the qjsc-output-raw branch July 4, 2024 07:31
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.

None yet

3 participants