bytecodealliance / StarlingMonkey

The StarlingMonkey JS runtime
Apache License 2.0
80 stars 11 forks source link

Task subclassing model #46

Open guybedford opened 4 months ago

guybedford commented 4 months ago

Fastly uses an AsyncTask model that stores on the AsyncTask class, the promise, and a function that will handle the process the "next steps" after the handle is ready, to then eventually resolve the promise.

On the other hand, StarlingMonkey only supports a singular run or cancel method on tasks.

So we have two separate task models in play:

  1. Fastly's task model, based on async handles, having "next steps" which resolve a promise ala spec algorithm descriptions
  2. StarlingMonkey's task model, where each subsystem subclasses their own tasks, and implements a single synchronous run function which is effectively those same "next steps", but with context and state specific to each subsystem subclass.

In adapting Fastly async tasks to this model, the easiest way to do this is to just subclass AsyncTask with FastlyAsyncTask that provides the original slots - [[Promise]] and [[NextSteps]], and then to use that like we had before.

This works fine, but I wanted to post this issue to see if there's more context on StarlingMonkey's async model that I'm perhaps missing.

guybedford commented 4 months ago

Alternatively, would it make sense for StarlingMonkey to define a generic PromiseAsyncTask which takes the callback function and creates and manages the promise automatically?

tschneidereit commented 2 months ago

@guybedford is this still relevant? (Apologies for the late response!)

guybedford commented 2 months ago

Our AsyncTask is defined here which does what I described in the original issue - https://github.com/fastly/js-compute-runtime/blob/main/runtime/fastly/host-api/host_api_fastly.h#L103.

We don't use the task subclassing at all, since we just treat all tasks as promises with contexts.

If you're open to a simplification like https://github.com/fastly/js-compute-runtime/blob/main/runtime/fastly/host-api/host_api_fastly.h#L103 then we could explore that, otherwise feel free to close as well, but wanted to give implementation feedback that we're not using the full expressivity of the model, and that feedback can go both ways if you think we can do things better.