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

Does a postTask API make sense for Node.js (in any way)? #33436

Closed
benjamingr opened this issue May 16, 2020 · 14 comments
Closed

Does a postTask API make sense for Node.js (in any way)? #33436

benjamingr opened this issue May 16, 2020 · 14 comments
Labels
discuss Issues opened for discussions and feedbacks. feature request Issues that request new features to be added to Node.js. timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout. worker Issues and PRs related to Worker support.

Comments

@benjamingr
Copy link
Member

benjamingr commented May 16, 2020

Hey, web browsers and framework vendors are working on a postTask API for dispatching tasks with particular priorities.

It is specified here and here is a basic example:

// https://github.com/WICG/main-thread-scheduling/blob/master/sample-code/scheduling-tasks.html
scheduler.postTask(() => 'This should be line 3', {priority: 'background'});
scheduler.postTask(() => 'This should be line 2', {priority: 'user-visible'});
scheduler.postTask(() => 'This should be line 1', {priority: 'user-blocking'});

// https://github.com/WICG/main-thread-scheduling/blob/master/sample-code/controlling-scheduled-tasks.html
// a taskController is also an AbortController which we are considering
const controller = new TaskController('background');
let result = scheduler.postTask(() => {}, {signal: controller.signal});
// control priority later
headerController.setPriority('user-blocking');

I opened an issue for Node.js use cases there. Copying it here:

I've been thinking of exploring adopting this API or experimenting with it in Node.js with the use case I've been thinking about is similar main thread contention. I have a server that serves a large number of users. That server needs to manage QoS of various levels of requests. Some requests should be very fast while for other requests I really wouldn't mind the users to wait a few milliseconds.

For example if I have a request that hashes an image and stores it for later retrieval - it might be OK for that request to wait until the CPU is idle. Work that takes more than a few milliseconds typically gets put in a (persistent) queue on servers but there is certain work I probably wouldn't mind waiting for.

The same goes for CLI tools, webpack might be interested in performing certain kinds of analysis in a lower priority than actually outputting the compiled code. TypeScript might "emit" before it's done type-checking to give users a snappier experience etc.

The main issue I see here is that Node.js already exposes threads and processes (with worker_threads and child_process) - so users already have access to the operating system scheduler. That said, it would be cool if some code could be made universal and shared here.


cc @shaseley, @sebmarkbage and @acdlite from the postTask side and @addaleax @BridgeAR @nodejs/workers @nodejs/open-standards from the Node side.


Is this interesting or relevant at all for Node.js and if so should we eventually strive to adopt it?

@benjamingr benjamingr added timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout. discuss Issues opened for discussions and feedbacks. worker Issues and PRs related to Worker support. labels May 16, 2020
@mcollina
Copy link
Member

I do not think this would be useful for Node.js like it is for browsers. In browsers, the JS vm has a lot of idle time, so it can process those tasks and keep the UI reactive. Note that delaying tasks increase memory usage.

In Node.js, we have either a lot of users or a single user that wants a response as soon as possible. In both cases, we should finish our compute as soon as we can.. and postponing tasks for the future increase the overall processing time because of the added gc costs :(.

I think Electron can benefit from this

@Bnaya
Copy link

Bnaya commented May 16, 2020

First, I think it's good for source code compatibility, even if it's mostly no-op it will make easier creating isomorphic code.
We can expect more and more libs to incorporate schedulable internals to run better on the browser and the authors will need to figure out how to expose that/configure that in node & browser.

Do we need that in nodejs for?
I agree that i would want to have scheduling for node for the described scenarios, especially the CLI ones. I can see myself want to process data for unknown time(The target machine CPU speed is unknown) but still want to be able to handle user input, file watchers, answer HTTP requests etc

Using scheduling can give me mush cheaper "threads" in terms of memory copying or manually share stuff using arraybuffers.
The code will need to be cooperative schedulable, so it's not flat drop in, but it gives the developer more to choose from to solve his use-case

@devsnek
Copy link
Member

devsnek commented May 16, 2020

I agree with mcollina, we don't really have anywhere to push low priority tasks to. In your server example, new requests coming in will continually displace your lower priority task and it will never run. The scheduler.yield api seems interesting but in terms of node it would have to be equivalent to await null, because anything with lower priority will never give control back.

@benjamingr
Copy link
Member Author

For reference here is the issue I opened in their repo WICG/scheduling-apis#17

I think a good next step would be for someone to look at whether or not Node libraries implement these sort of schedulers in the same way frontend libraries do or not.

@Fishrock123
Copy link
Contributor

Fishrock123 commented May 19, 2020

I suppose one could have a type of libuv task that only gets called if the rest the libuv tasks return nothing from poll, but generally it's very hard to estimate this properly.

I suppose in practically doing it "properly" doesn't matter, as long as the user understands that it cannot be estimated exactly and that the task still blocks polling while it is run.

(Also, not sure this really has anything to do with "timers".)

@benjamingr
Copy link
Member Author

(Also, not sure this really has anything to do with "timers".)

The postTask API has an optional "delay" parameter, effectively making it a timer. This might have further implications if the the delay is tied to a priority and can't be implemented as a setTimeout + postTask at that priority.

@Fishrock123
Copy link
Contributor

Fishrock123 commented May 19, 2020

If it can't be implemented that way your delayed task might actually be less efficient overall, lol.

@benjamingr
Copy link
Member Author

Here is my concern: we have a history of not collaborating too well with browsers and spec bodies on APIs (fetch, abortcontroller, streams, promises etc) and then being frustrated when APIs users ask for don't make sense for node.

If browsers implement this in all likelihood users will start asking for it in a year or two. If we have concrete feedback API wise or quality wise then we should provide it.

Otherwise it might play out like fetch where:

  • We say "this doesn't make sense for Node" and ignore it for a year or two.
  • It becomes popular and users start using it.
  • We have a lot of outstanding issues with it and can't easily add it to Node

For example - we're probably going to have issues with TaskController (like we do with AbortController today).

@jasnell
Copy link
Member

jasnell commented May 20, 2020

Hmm.. might be a good time to point people to Piscina... A worker thread pool module that @addaleax and I have been working on for the past month. We've done a significant amount of analysis on various workloads and performance and will be publishing a blog post soon about it.

The module supports a runTask API, implements various concurrency and queue options, and supports a priority queue mode, among several other things. We could likely, fairly easily, build a version of this proposed browser API on top of it but there are countless ways where the processing semantics aren't going to be even nearly the same.

@jasnell
Copy link
Member

jasnell commented May 20, 2020

One strategy for implementing this would be to add a check handle to the libuv queue that, when invoked, is set to run for a target period of time. During that time, it pulls callbacks from a priority queue and executes them. If, after it executes one callback, the target time has elapsed, it returns control to the event loop. If the time hasn't yet elapsed, it picks the next item and invokes it. However, because we can't actually guarantee the order of check handles in the loop (user code could add more), inserting this as a permanent immediate that runs at the start of each loop turn would be necessary... it would essentially become a setImmediateWithPriority() type of API.

The performance benefits of this (versus using a worker thread pool) are... questionable... but could be easily explored using setImmediate() and a priority queue (the shuffled-priority-queue module works really well). I don't think the implementation would be too difficult but obviously there are details to work through. I really question whether there's going to be any actual benefit to the API tho. It could actually just have the effect of reducing overall performance.

@jasnell
Copy link
Member

jasnell commented Jun 19, 2020

Thinking about this further, I'm -1 on adding this in core at this time. It could make for a useful userland module and could easily be implemented as such.

@jasnell jasnell added the feature request Issues that request new features to be added to Node.js. label Jun 19, 2020
@benjamingr
Copy link
Member Author

To be fair, I don't actually understand what the API does - if it's just sugar and doesn't map logically to Node I wholeheartedly agree.

@Fishrock123
Copy link
Contributor

Fishrock123 commented Sep 2, 2020

I would hazard a guess it is useful in some way regarding frame updates in the browser GUI, although I don't know for certain.

@benjamingr
Copy link
Member Author

I think discussion has stalled so I'll close this and if the project ever decides it's a good idea let's reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Issues opened for discussions and feedbacks. feature request Issues that request new features to be added to Node.js. timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout. worker Issues and PRs related to Worker support.
Projects
None yet
Development

No branches or pull requests

6 participants