bytecodealliance / javy

JS to WebAssembly toolchain
Apache License 2.0
2.06k stars 100 forks source link

Remove usage of the custom `rquickjs` fork #675

Closed saulecabrera closed 1 week ago

saulecabrera commented 2 weeks ago

This commit deprecates the usage of the custom rquickjs fork and pins the rquickjs version to 0.6.1, which is the current version in the fork.

The main motivation behind this change is to avoid relying on a fork, which precludes publishing version 3.0.0 of the javy crates to crates.io, since git dependencies are not supported.

Another motivation, arguably more important than the one stated above, is making sure that we are always relying on the upstream implementation rather than in a fork containing patches maintained out-of-band.

Side-effects:

The fork enabled a slightly more performant execution in Wasm environments, when dealing with short-lived JS programs that don't require garbage collection, more explicitly, it introduced a patch that prevented QuickJS' __JS_FreeValueRT from running. Even though there are no known issues with this approach, __JS_FreeValueRT is an internal method and no guarantees are made regarding its stability, therefore it's preferred to avoid relying on it. Without this custom patch, I'm observing locally, on average a 2% increase in fuel usage outside of the 10% acceptable variance. Note that there are safe mechanisms in place to mitigate the impact of GC in the codebase, namely: (i) using ManuallyDrop where possible (ii) setting the GC threshold to -1 to reduce the possibility of any GC pauses. With that in mind, the 2% increase seems reasonable given all the benefits that we get from this change.

Alternatives considered:

Checklist