Reuse runtime when compiling source code to bytecode

[jeffcharles]

Description of the change

Re-use the runtime when compiling JS code to QuickJS bytecode. Also update the dylib test code to refresh the instance to work around the module being evaluated twice.

An unfortunate result of this change is the a fresh instance would need to be used after invoking compile_src if invoke were called were called with a JS function. I don't think anyone would be relying on this behaviour. I'm also not entirely why we're seeing this behaviour. If I update invoke to not call execution::run_bytecode, everything runs as expected if a fresh instance isn't created. However, if a fresh instance is used (as in the common use case), then QuickJS indicates the module has not been declared and traps.

Why am I making this change?

Part of #768. If a plugin were to change any JS runtime configurations and if any of those configurations were to effect QuickJS bytecode emission, we would want to use that runtime configuration when compiling the QuickJS bytecode. Without this change, a default runtime configuration would be used instead which may not be intended.

An alternative approach is to persist the configuration generated in wizer.initialize and use that configuration when initializing the runtime in compile_src. This approach will not continue to work if we were to add the ability to further customize the runtime in wizer.initialize (which is planned).

I also looked into if there was a way to "de-register" a JS module from a QuickJS context but couldn't find a way to do it. Hypothetically I could re-register an empty module with the same name after compiling the bytecode which may have a similar effect.

Checklist

• I've updated the relevant CHANGELOG files if necessary. Changes to javy-cli and javy-core do not require updating CHANGELOG files.
• I've updated the relevant crate versions if necessary. Versioning policy for library crates
• I've updated documentation including crate documentation if necessary.

[saulecabrera]

The code changes LGTM. However, I think I'm a bit conflicted by the open questions and assumption around the change.

If a plugin were to change any JS runtime configurations and if any of those configurations were to effect QuickJS bytecode emission

Do we have any evidence of which configurations of the ones that we allow might affect compilation to bytecode? Naively, I assumed that most of those features wouldn't change compilation and only affect the runtime behavior. But maybe that assumption is not correct.

An unfortunate result of this change is the a fresh instance would need to be used after invoking compile_src if invoke were called were called with a JS function.

It's probably unlikely that anyone is relying on this; however, if we end up settling on this approach, it might be worth considering adding some documentation around it, which I suspect will require a bit more of investigation to get to the bottom of the root cause.

[jeffcharles]

Do we have any evidence of which configurations of the ones that we allow might affect compilation to bytecode?

I have evidence when changing the bignum_extension intrinsic. I am making an assumption that we would want to make that and other underlying intrinsics configurable by plugins.

Given a JS program that looks like this:

function foo() {
    "use math"
    1234 % 32
}

With config.bignum_extension set to true, QuickJS produces the following bytecode:

C\02\18function.mjs\06foo\0d\c8\03\00\00\00\00\00\0c \06\01\a4\01\00\00\00\01\01\01\09\00\ca\03\00\01\08\ec\05\c2\00\e3)\06.\c8\03\01\04\01\00\07\08\00\00\0cC\06\05\ca\03\00\00\00\02\00\00\07\00\c0\d2\04\bf \b4)\c8\03\01\03\03\12\12\00\06\00\00\08\00\03\0e

With config.bignum_extension set to false, QuickJS produces the following bytecode:

C\02\18function.mjs\06foo\0d\c8\03\00\00\00\00\00\0c \06\01\a4\01\00\00\00\01\01\01\09\00\ca\03\00\01\08\ec\05\c2\00\e3)\06.\c8\03\01\04\01\00\07\08\00\00\0cC\06\01\ca\03\00\00\00\02\00\00\07\00\c0\d2\04\bf \9d)\c8\03\01\03\03\12\12\00\06\00\00\08\00\03\0e

These are slightly different. The 164th, 209th, and 210th characters are different.

It might be worth considering adding some documentation around it, which I suspect will require a bit more of investigation to get to the bottom of the root cause.

Adding documentation around the limitation makes sense. I could spend more time looking into the root cause but alternatively, we could also just say it's not supported right now and if someone complains, we can look more into exactly what's going on at that point.

[saulecabrera]

Thanks for investigating this and for your patience here. I think it might make sense to include the results of your findings as part of a comment in the code.

Adding documentation around the limitation makes sense. I could spend more time looking into the root cause but alternatively, we could also just say it's not supported right now and if someone complains, we can look more into exactly what's going on at that point.

The part that I struggle the most with is the "fresh instance" part of the workaround, however, I do agree that monitoring and digging deeper when/if more issues arise is probably best.

[saulecabrera]

Could you add some docs that include your comment?

[jeffcharles]

The part that I struggle the most with is the "fresh instance" part of the workaround, however, I do agree that monitoring and digging deeper when/if more issues arise is probably best.

The current constraint is that invoke can't use the runtime that compile_src used. Generating the bytecode for the module in compile_src causes the module to be in a declared state in the runtime. eval_bytecode seems to not evaluate that declared module for some reason but instead evaluates a copy of the same module. Then, I think, invoke evals the version of the module that was declared when running compile_src and not the one declared as part of running eval_bytecode. I'm basing that on, I can delete the eval_bytecode call and not pass any bytecode to invoke and everything runs successfully when calling invoke (but this only works when using the same runtime compile_src was run with).

[jeffcharles]

I experimented a bit with how we're loading the bytecode and I think I have a solution that will both solve the problem about not being able to re-use the instance/runtime and also reduce the instruction count when calling an exported function. I'll make that PR separately and then rebase this PR on top of that one.