-
Notifications
You must be signed in to change notification settings - Fork 114
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Remove usage of the custom
rquickjs
fork (#675)
* Remove usage of the custom `rquickjs` fork This commit deprecrates 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 introuced 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: * Keeping the fork * Upgrades will be difficult * There's no easy way of publishing the `javy` crate * Post-processing the generated Wasm code and make `JS_FreeValue` and `JS_FreeContext` no-op. * This worked to a certain extent, however, in my opinion, the result is not worth the effort. This approach resulted in minimal performance improvements. Even though `JS_FreeValue` which is a method from the public API and is invoked by Rust `drop`s will end up calling `__JS_FreeValueRT`, which is the most expensive of the `free` variants, not all `__JS_FreeValueRT` invocations stem from `JS_FreeValue`, some of them happen automatically from within the JS code. * Cargo vet * Use `&Ctx<_>` in 262 tests * Fix integration test
- Loading branch information
1 parent
ef05dbd
commit 93df556
Showing
9 changed files
with
308 additions
and
94 deletions.
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.