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

Consider reusing AsyncContext specification infrastructure for "current scheduling state" #94

Open
littledan opened this issue Jun 25, 2024 · 6 comments

Comments

@littledan
Copy link

littledan commented Jun 25, 2024

The Stage 2 JavaScript AsyncContext proposal adds state which is saved and restored across await, like the current scheduling state. Can we model the current scheduling state as an AsyncContext.Variable instance which is not exposed to JavaScript, just held internally by HTML? If so, then it would allow implementations to share the same code paths, reducing overall complexity.

Note that there is a currently open design discussion about how AsyncContext is saved and restored across various web APIs (issue, html issue). I'm curious whether the propagation of the current scheduling state should be propagated in the same way.

@shaseley
Copy link
Collaborator

Would there be a way to limit the propagation of an AsyncContext.Variable to just continuations/promises, rather than propagating across various web APIs? For the "yieldy" APIs like scheduler.yield(), we wanted to propagate the state through the "yieldy async task" (task chain) vs. the entire task graph that might be created from the task, which IIUC is where AsyncContext differs.

@littledan
Copy link
Author

Do you have any information about the motivation for not propagating across the entire task graph (e.g., why not also to setTimeout and event handlers)? Also, I'm curious what the priority is set as, for things which do not inherit this way.

For AsyncContext, we'll probably have one main location in WebIDL where we propagate the AsyncContext.Snapshot to most (all?) callbacks. We could, if needed, add logic in here to set the scheduling state to a particular thing at that point, when calling the callback, if they are all supposed to be considered as a "fresh" task with a different priority.

@shaseley
Copy link
Collaborator

Do you have any information about the motivation for not propagating across the entire task graph (e.g., why not also to setTimeout and event handlers)?

Sure. Our goal was for the scheduling state to be propagated through the body of an async function, such that breaking it up with scheduler.yield() would retain the abort/priority state. For example:

const task = async () => {
  while (hasWork()) {
    doSomeWork();
    // The continuation inherits the task's priority.
    await scheduler.yield();
  }
};

const controller = new TaskController({priority: "background"});
scheduler.postTask(task, {signal: controller.signal});

Conceptually, when breaking up a long synchronous task with scheduler.yield(), we're taking

[--------------------------- task ---------------------------]

and turning it into

[-- task start --] [-- continuation --] [-- continuation --]

and propagating the state to all the pieces. Things of course get more complicated if you mix in other promise-based async APIs, which is discussed a bit here.

In my mental model, if scheduler.yield() is called within setTimeout(), it's conceptually a continuation of that timer task — not the task that scheduled it. And while there is a relationship, IMO it's not clear how the priority/abortability of continuations that happen within the setTimeout relate to the originating task.

For example, suppose a postTask task calls into a third party library or triggers a framework update, either of which might call setTimeout(), rAF, or postMessage(). And suppose those spawn other tasks which eventually call scheduler.yield(); is the yield() a continuation of original task?

const task = () => {
  doSomeWork();
  // This might schedule yieldy tasks via setTimeout(). Is that a continuation of this?
  informThirdPartyLibrary();
  // This might trigger framework updates and eventually lead to custom hooks.
  // Are all of those related?
  triggerFrameWorkUpdate();
};

A risk with inheriting broadly across the task graph is "runaway prioritization": elevated priority can be effective, but prioritizing everything in the task graph can lead to over-priortization. Reducing the scope such that yield() is bound to the innermost task (rather than most recent prioritized ancestor task) helps mitigate this risk.

On the other hand, there might be cases where propagating an AbortSignal or TaskSignal across the entire graph is what developers want (including by default in scheduler.postTask(), which currently doesn't support inheritance). I think it could be worthwhile to explore APIs that support this, but I think it's beyond the scope of scheduler.yield() inheritance.

Also, I'm curious what the priority is set as, for things which do not inherit this way.

The scheduling state is only set (and propagated) for scheduler.postTask() (based on the arguments) and requestIdleCallback (no abort, "background" priority). For other APIs, including setTimeout(), nothing gets propagated. In those cases, the default priority ("user-visible") is used (step 8 of this algorithm).

@littledan
Copy link
Author

littledan commented Jul 1, 2024

I wonder if this concern about how "scheduler-like" APIs should reset priority implies to other things which may be tracked by AsyncContext variables, in case they're run out of an unsuspecting third-party library. I suppose they definitely want task attribution propagation, but (for example) if we implicitly propagated an AbortSignal for cancellation, we would probably want that to not propagate, if the third-party scheduled tasks are somehow logically detached (which I assume is the motivation for not propagating the priority).

Are your thoughts on this topic influenced by any particular JS code that you've examined? It'd be helpful to open that back up to understand the implications on AsyncContext more broadly.

@mmocny
Copy link
Collaborator

mmocny commented Jul 3, 2024

I had a thought: If the AsyncContext model of propagation would retain the scheduling options more than desired via certain scheduling mechanisms, could we just explicitly reset these in those places?

E.g., setTimeout would by default retain its AsyncContext that holds the previous task priority or abort signal, but the scheduler could choose to override those to reset them because setTimeout doesn't inherit.

If I were implementing a userland scheduler, that might be how I would do it.

Conceptually, I think it could be a way to merge the two concepts (AsyncContext specifies all the ways in which context is propagated, and task scheduling chooses to constrain down / drop the context sometimes). In practice I'm not sure if that is how it would be implemented most efficiently.

@andreubotella
Copy link

I've now opened tc39/proposal-async-context#100, with a document outlining the web integration of AsyncContext in detail.

I had a thought: If the AsyncContext model of propagation would retain the scheduling options more than desired via certain scheduling mechanisms, could we just explicitly reset these in those places?

Yes, that would be possible, but the handling of callbacks in WebIDL would need to take the scheduler.yield variable into account, which might not be great for performance.

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

4 participants