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

Future plans for QuickJSR? #7

Open
wch opened this issue Apr 12, 2024 · 6 comments
Open

Future plans for QuickJSR? #7

wch opened this issue Apr 12, 2024 · 6 comments

Comments

@wch
Copy link

wch commented Apr 12, 2024

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-quickjs

Then 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/sassy

This 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?

@wch wch changed the title Future plans for QuickJSR Future plans for QuickJSR? Apr 12, 2024
@andrjohns
Copy link
Owner

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

@andrjohns
Copy link
Owner

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

@wch
Copy link
Author

wch commented Apr 14, 2024

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. When for loop has a label at beginning, break does not work correctly bellard/quickjs#275

@andrjohns
Copy link
Owner

@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?

@wch
Copy link
Author

wch commented May 7, 2024

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.

@bnoordhuis
Copy link

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.)

I fixed that today in quickjs-ng/quickjs@66732e7 (and quickjsr already uses quickjs-ng, I believe?) so you should be able to remove those workarounds after the next upgrade.

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

No branches or pull requests

3 participants