Open wch opened 7 months ago
Great to hear that the package could be useful!
I do have plans to expand the package's functionality, with the end goal of being a drop-in alternative for V8 (where possible), as well as improve the documentation and vignettes.
Unfortunately I'm a bit short on spare time at the moment, so it will be another month or two before I'll be making any large changes.
I'm happy to take a look at any smaller bugs/issues/requests in the interim, so feel free to let me know if anything comes up
Also as an aside, you could avoid bundling the quickjs
sources/engine by just adding LinkingTo: QuickJSR
in your package description.
This will save you some headaches with CRAN, since the QuickJS C code causes several -Wpedantic
warnings. The bundled sources/headers here have several patches for these. I have them all in a fork here, such that they're only applied when the -DSTRICT_R_HEADERS
flag is passed
Good to hear that you're planning on continuing work on this package.
Some thoughts:
I think that the package would need Imports: QuickJSR
instead of LinkingTo
.
I think the JSContext
class should be created with cloneable=FALSE
(making cloneable=TRUE
the default for R6 was a mistake but one that is very hard to change at this point).
It would be convenient if the qjs_eval()
function had a file
parameter, if you want to run JS code from a file.
To speed things up, it would be nice to be able to provide pre-compiled bytecode to QuickJSR, or have QuickJSR compile JS code and then cache the resulting bytecode somewhere for later runs. I found that when I ran the qjs
interpreter, it takes about 0.3s to run the following command with sass.js:
❯ time qjs sass.js examples/test.scss > /dev/null
qjs sass.js examples/test.scss > /dev/null 0.28s user 0.01s system 95% cpu 0.305 total
However, if I compile it to a binary and then run that, the total time is about 0.9 seconds.
❯ qjsc -o sass sass.js
❯ time ./sass examples/test.scss > /dev/null
./sass examples/test.scss > /dev/null 0.07s user 0.01s system 90% cpu 0.088 total
I believe the time difference is probably because qjs
parses the JS file and compiles it to bytecode, whereas the compiled binary has already done those steps. It would be nice to be able to provide compiled bytecode to QuickJSR so that it can run without the parsing/compiling overhead. I think that probably could be done by passing raw vectors of the bytecode to a C++ function. However, one problem with this is that there's probably no guarantee that bytecode generated by one version of QuickJS will run on another version, so we would need to ensure that the bytecode was generated by the version of QuickJS that's in this package. Maybe QuickJSR could compile to JS to bytecode the first time it's run, and then put that bytecode in a persistent cache somewhere?
Ideally there would be minimal external dependencies for QuickJSR. You might expect me to be biased in favor of keeping R6 (as its author 😄), but I believe the use here could be replaced with a function that returns a list of closures. I think that it would be better to use cpp11 instead of Rcpp, because it is a compile-time-only dependency, and it is simpler in many ways. It would be significantly more work to replace jsonlite, but it should be possible to do data interchange without serializing to JSON on one side and then deserializing it on the other. I recognize that would be a big project, though, and possibly not worth the effort.
What do you think about changing the name to all lowercase, like quickjsr
? Or maybe even just qjs
?
I found a bug in quickjs which prevented the Dart Sass compiler from working. (I had to add some workarounds in the Dart Sass code to make it work.) Hopefully they will fix it soon and make a new release. https://github.com/bellard/quickjs/issues/275
@wch Thanks for all of these suggestions, and I agree! I'll be starting on updates this week, so will be working through these.
What do you think about changing the name to all lowercase, like quickjsr? Or maybe even just qjs?
I wouldn't mind renaming to quickjsr
, but is that possible with CRAN? I didn't think they allowed renaming packages?
That's great to hear!
I guess renaming it to quickjsr
might not be worth the trouble, and could cause problems for users with case-insensitive filesystems (Mac and Windows), if they try to install the old and new packages. Although, the name qjs
might be good. I don't feel very strongly about it either way.
Hi, I'm curious: what are your future plans for QuickJSR?
The reason I ask is because I have been experimenting with using JS in R packages, and quickjs has some important advantages over V8 for use with R.
Some background: the sass package wraps the C libsass library, but libsass has been deprecated for several years now, and we are looking for a way forward.
I believe that the only Sass compiler implementation that's being developed these days is Dart Sass, which can be compiled to JavaScript. I wrote a JS wrapper for Dart Sass (compiled to JS), which can be run from the command line using the
qjs
runtime: https://github.com/wch/sass-quickjsThen I used
qjsc
to generate a C file for this, and put that in an R package, so that it can be used from R: https://github.com/wch/sassyThis is OK, but result is fairly large -- the built binary package is a 2.2MB tarball, and the vast majority of that is the QuickJS runtime. If we were to use JS in any other R package and embed it this way, it would be another few megabytes for each packge.
That brings me to QuickJSR. I can see that QuickJSR can be used to evaluate JS code, but if we were to use it, we would want to be sure that it would be developed and supported in the future. Are you planning on continuing development on QuickJSR?