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

Introducing QuickJS engine as alternative JS engine. #698

Open
5 of 14 tasks
xeioex opened this issue Mar 6, 2024 · 13 comments
Open
5 of 14 tasks

Introducing QuickJS engine as alternative JS engine. #698

xeioex opened this issue Mar 6, 2024 · 13 comments

Comments

@xeioex
Copy link
Contributor

xeioex commented Mar 6, 2024

QuickJS will be added as an alternative JS engine. The goal is to achieve a drop-in replacement level support for QuickJS.
QuickJS is supported and developed by a wider community, it has almost full ES spec coverage.

QuickJS will be added in stages:

core modules:

The following items will not be ported:
njs-specific API: njs.dump(), njs.on, console.dump().
js_preload_object directive for http and stream.

NJS engine will stay a default JS engine for a while for backward compatibility (at least until QuickJS version is mature enough).

Feel free to ask questions and share you feedback.

@radsoc
Copy link

radsoc commented Mar 8, 2024

I am very concerned about this move.
It contradicts everything that was said in your 2018 talk (https://youtu.be/Jc_L6UffFOs), which I fully support.
Performance is key; full ES spec coverage is not; njs is not a nodejs replacement.

vlcsnap-2024-03-08-10h23m13s249
vlcsnap-2024-03-08-10h29m04s985
vlcsnap-2024-03-08-10h06m45s269
vlcsnap-2024-03-08-10h52m30s465
vlcsnap-2024-03-08-10h33m30s382

QuickJS and Ducktape are in the same ballpark.

2024-03-08_10-17

And QuickJS is garbage collected.

2024-03-08_10-38

@xeioex
Copy link
Contributor Author

xeioex commented Mar 9, 2024

Hi @radsoc,

Thank you for the feedback and for letting us know that NJS is preferred. Let me address some of your questions.

Performance is key; full ES spec coverage is not; NJS is not a Node.js replacement.

When NJS was introduced, we compared it to existing engines, like Duktape, and found that NJS was faster. QuickJS, introduced in 2019-2020, is actually fast, on a level with NJS, and even 2x times faster in some benchmarks, despite its relatively costly GC. (We are currently addressing the performance gap in NJS.)

A custom interpreter can be tailored to the NGINX runtime.
Each request should be isolated from others.

The first statement is still true; NJS is definitely faster for short code snippets because it is quite lightweight for creating/destroying instances. It would be 10x times slower to do this in QuickJS, mostly because QuickJS is not optimized for fast instance creation.

To summarize, NJS is still better for the use cases and principles for which it was designed.

At the same time, the situation is different when more people write increasingly complex scripts. Usually, they do not want to write their own scripts; they tend to reuse existing JS libraries. They also want more advanced scenarios for JS scripting in NGINX, like long-lived Workers with persistent JS instances. Since NJS has no GC, this use case is impossible with NJS.

When people reuse existing libraries, their code is quite large, and GC and performance become more important than fast instance creation.

With QuickJS, we want to address the limitations NJS has and compare it with the alternatives. If there is feedback and NJS is still a better option for some use cases, then we will preserve it.

@radsoc
Copy link

radsoc commented Mar 9, 2024

Thank you for this explanation @xeioex.

But I don't see the point in using NGINX for long-lived Workers.
For this scenario, V8 and JSC (node/deno or bun) are 30x faster than QuickJS.

So yes, I'm all in favor of preserving NJS.

@lancedockins
Copy link

lancedockins commented Apr 9, 2024

@xeioex I definitely have some concerns about this as well.

As @radsoc mentioned, Nginx isn't going to function like a browser with long-lived JS contexts that might have occasion to run very lengthy processes. Unless you're streaming a video or something, the entire request lifecycle for an NJS context is going to be supremely short.

It sounds like what you're saying is that QuickJS is as fast or faster at processing JS code once the context is instantiated but is 10x slower at instantiating contexts. If that is correct, then you're basically saying that NJS is about to become 10x less scalable than it currently is due the the fast, short-lived, and voluminous context needs that it has for real-world traffic. If that understanding is correct, you really can't just write that off like it isn't a HUGE problem. Nginx is used on extremely high-traffic sites all of the time. In fact, we use it on some very high-traffic sites and across hundreds of servers.

As this has been described so far, it really sounds like NJS has decided to abandon its core purpose just to quiet a few complaints on the GitHub issues list. I fully understand that people "want" to use off-the-shelf Node modules. That certainly would have made our job a lot easier when we were building our NJS projects. But a lot of Node modules are built for entirely different environments and have a ton of crap in them that really has no business being in a web server scripting solution. There's just no world in which scalability and stability are worth exchanging for "cool" tech that really just silences people who haven't thought out the consequences of doing whatever they're proposing in a web server environment.

What is NJS doing to speed up context instantiation to ensure that the actual use case (a web server) isn't just going to get oblierated by the shift to QuickJS instead?

@xeioex
Copy link
Contributor Author

xeioex commented Apr 10, 2024

@lancedockins

Thank you for the feedback.
First of all at this moment we do not plan to retire njs engine.

What is NJS doing to speed up context instantiation to ensure that the actual use case (a web server) isn't just going to get oblierated by the shift to QuickJS instead?

We plan to address this issue by using an single QuickJS instance per location and not per each request as NJS does. This solves the problem of slow instantiation. The GC should prevent instance memory from exploding. It introduces potential memory leaks in certain cases though, but it should work in many cases just fine.

@lancedockins
Copy link

lancedockins commented Apr 10, 2024

@xeioex That's an interesting approach. But I think that it does raise a few other questions.

  1. With a single instance per location, wouldn't we be talking about persistent VMs instead of on-demand VMs for each request?
  2. If we're talking about persistent VMs for each location (rather than a single persistent VM that is just constantly re-cloned from its initial instance), wouldn't that lead to both higher resting memory use in Nginx for servers with a lot of locations? Possibly even significantly higher resting memory use in order to store all of those location-specific VMs?
  3. And wouldn't it raise the possibility that you could run into some sort of VM state corruption that can't exist right now with a per-request instance? Since VM's are per-request now, you never have to worry about the state of the VM at the end of the request because NJS just throws out the VM and clones the base VM for the next request. So the environment is fresh on every new request. But if we're talking about persistent VM's per location with QuickJS, it seems like the VM's would not be dropped at the end of the request and that would require greater care with your VM states since the next request to that location would just inherit the persistent VM state from wherever things left off for that location VM at the end of prior requests.
  4. Have you run any benchmarks for scale comparison on QuickJS in the configuration that you've described vs NJS? I think it might alleviate some of the concerns if benchmarks of QuickJS VM instantiation in the way that you're describing is at parity with or better than what you're already doing with NJS
  5. Given that I've personally run into memory leaks with NJS already, it's concerning that the new strategy might actually introduce more of such leaks. I'd certainly hope that the net result of the change would be lower resting and runtime lower memory as well as memory growth over time. But as you're describing it, I don't see how that would be possible.

@xeioex
Copy link
Contributor Author

xeioex commented Apr 10, 2024

Hi @lancedockins,

The items 3 is certainly a possibility. There is a trade-off between:

  • having persistent memory which quite often is desired.
  • having a clean environment for each request.

The first allows to implement various cross-requests logic (caching, stat counters). Also it allows to use large routing tables.
On the con side this scheme is more vulnerable to suboptimal JS code which can produce non garbagrable leaky structures.
In most cases, if you do not do it on purpose, you will do fine.

The second is fast at destruction/creation and is better protected from poor JS code (not bullet-proof though). On the con side: the code has to read for each request its own data from file or other sources, which can be slow.

Have you run any benchmarks for scale comparison on QuickJS in the configuration that you've described vs NJS?

I did some benchmarks, but in a standalone setup, not as a part of nginx.

I'd certainly hope that the net result of the change would be lower resting and runtime lower memory as well as memory growth over time

As a said at the beginning we will not make QuickJS default engine until it is mature enough in terms of speed and functionality. We also do not plan to retire NJS.

agebhar1 added a commit to agebhar1/system-design-playground that referenced this issue Jun 15, 2024
agebhar1 added a commit to agebhar1/system-design-playground that referenced this issue Jun 15, 2024
Support for QuickJS is in progress [1].

[1]: nginx/njs#698
agebhar1 added a commit to agebhar1/system-design-playground that referenced this issue Jun 15, 2024
agebhar1 added a commit to agebhar1/system-design-playground that referenced this issue Jun 15, 2024
Support for QuickJS is in progress [1].

[1]: nginx/njs#698
agebhar1 added a commit to agebhar1/system-design-playground that referenced this issue Jun 15, 2024
agebhar1 added a commit to agebhar1/system-design-playground that referenced this issue Jun 15, 2024
Support for QuickJS is in progress [1].

[1]: nginx/njs#698
agebhar1 added a commit to agebhar1/system-design-playground that referenced this issue Jun 15, 2024
agebhar1 added a commit to agebhar1/system-design-playground that referenced this issue Jun 15, 2024
Support for QuickJS is in progress [1].

[1]: nginx/njs#698
@s3rj1k
Copy link

s3rj1k commented Jul 15, 2024

@xeioex Can we just have a separate module for NJS runtime and for QuickJS runtime and maybe call it differently, NJS in current state is really just a scripting language but QuickJS will possibly be a fullblown application framework, how this would be different from lua nginx modules in terms of targeted audience?

@xeioex
Copy link
Contributor Author

xeioex commented Jul 17, 2024

Hi @s3rj1k,

The plan is to introduce QuickJS as a drop-in replacement for NJS engine. So most of the existing scripts and configurations should work with the new engine.

To clarify, QuickJS is just an JS engine as NJS. By itself it implements JS ES6 and beyond standards. In effect it will be similar to NJS as a scripting language.

@kaiwu
Copy link

kaiwu commented Aug 6, 2024

All hands up for QuickJS option, full ES6 compatibility will prompt NJS to another level. Big thanks for this caring alternative. If it turns out, QuickJS is on par with Lua performance wise, it would be a HUGE win.

@bnoordhuis
Copy link

Hey all, I'm wondering if you've given consideration to switching to quickjs-ng instead? I don't want to speak for Charlie and Fabrice but I believe it's fair to say they've other things going on and that development of bellard/quickjs has kind of stalled (again) as a result.

As a couple of data points: radareorg/radare2 recently switched to quickjs-ng, awslabs/llrt is switching as we speak, and so is rquickjs, the Rust language binding. I fully expect more projects to follow.

(My selfish reason as a quickjs-ng maintainer for promoting it here is that I want to have more users and more use cases, to help me understand in what areas I can most fruitfully invest my time.)

@xeioex
Copy link
Contributor Author

xeioex commented Oct 28, 2024

Hi @bnoordhuis,

We do not have specific plans yet, but given current circumstances with the stalled development, I think we will have to migrate to the quickjs-ng in the near-term future. Especially, if we will encounter a bug that have to be fixed in QuickJS that affects us. Ideally, I would prefer the staying with the original repo.

What about the previous plans to merge quickjs and quickjs-ng?

@bnoordhuis
Copy link

We've had some preliminary talks before the summer but nothing concrete came out of that. For the foreseeable future, quickjs-ng is where it's at.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants