sebastienros / jint

Javascript Interpreter for .NET
BSD 2-Clause "Simplified" License
4.04k stars 558 forks source link

async/await and callback support #514

Open christianrondeau opened 6 years ago

christianrondeau commented 6 years ago

I was surprised to find no issues nor pull requests to support C#/JS async/await and Task support.

In a perfect world, I could make that transparent and "halt" the calling method (like #192), or use a callback mechanism (which would make scripts much more complex however).

I'd prefer to avoid using return task.Result and friends, so that I don't get into a deadlock eventually.

Before investigating this further, did anyone actually attempt any async/await support in Jint? @sebastienros is this something you'd be interested in supporting, e.g. through the native ECMA-262 async/await (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/async_function)? Any failed or partial attempt?

To avoid any confusion, given this C# code:

public async Task<string> loadSomething(string path) {
  return await File.ReadAllTextAsync(path);
}

I'd consider any of these JavaScript equivalents to be correct (in order of preference):

// Pausing (easiest for script writers):
var x = loadSomething('test.txt');
// Async Function (concise)
var x = await loadSomething('test.txt');
// Promises (well known)
var x;
loadSomething('test.txt').then(function(result) { x = result });;
// Callbacks (worst case)
var x;
loadSomething('test.txt', function(result) { x = result });;

Any ideas or feedback on that is welcome; I'd like to avoid spending any time on this if it would not be merged, or if the effort is too large for what I can afford.

ackava commented 6 years ago

Can you not convert simple callback into promise using following?

     public class AsyncService {
          public void LoadAsync(JValue input, JValue then, JValue failed) {
                 Task.Run(async () => {
                         try {  
                                 var result = await ..... async processing in C#
                                 then.Invoke( null, result );
                         } catch(Exception ex) {
                                 failed.Invoke(null, ex.ToString());
                         }
                  });
          }      
     }

             loadAsync( input ) : Promise<any> {
                  return new Promise((resolve,reject) => {
                           asyncService.loadAsync( input, resolve, reject);
                  });
             }

     // now you can use loadAsync method with async await
christianrondeau commented 6 years ago

@ackava thanks for sharing your ideas. It looks like you are using TypeScript, which would explain why using the Promise constructor, the => syntax and eventually using await would work for you :) Using a transpiler is an idea worth investigating though.

I realize that supporting async/await would pretty much be supporting ES2017, which has it's own open issue: https://github.com/sebastienros/jint/issues/343 - this would probably (I can only guess) solve my issue, depending on how @sebastienros decided to implement "await" for C#-backed functions. For now, the Promise constructor looks like it's not implemented in the 3.0 branch, so I guess we'll have to wait and see. If I had more time, I'd try contribute this back in Jint, but I don't think I'll be able to take the lead on this for some time.

If someone else wonders how to do transpiling in Jint: https://github.com/sebastienros/jint/issues/274 (did not try it yet)

ta10212 commented 6 years ago

Hi everyone, I echo @christianrondeau in the desire for async/await support.

While promises would work, it would be even simpler if the Engine could just "await" a function delegate passed to "SetValue" - if that delegate wrapped an asynchronous method.

Suppose we had the following C# code:

Engine engine = new Engine(cfg => cfg.AllowClr());
Func<string, Task<string>> thisFunc = DoOperation;
engine.SetValue("DoOperation", thisFunc);

//C# method of DoOperation
public async Task<string> DoOperation(string input)
{
   //assume that SomeAsynchronousOperation returns a string...

   return await SomeAsynchronousOperation();
}

The ideal scenario would then be for an "ExecuteAsync" method on the engine like:

engine.ExecuteAsync("DoOperation('my_input');");

...where internally the engine would "await" the function delegate at the time of invocation.

@sebastienros , is this at all possible? If not, are there any other workarounds you might suggest?

Jint is a phenomenal piece of software though in my particular case, I'm dealing with heavy network I/O. As such, async/await support is critical.

ellern commented 5 years ago

Has anybody had any progress on this issue that's worth sharing? I'm in a situation where I need to call some async methods.

jbourny commented 5 years ago

Hello, I've maybe a solution for you, it doesn't use await syntax but you can use then(...) :

C# function to call from JS :

public static Promise<bool> alert(string msg)
        {
            return new Promise<bool>(Task.Run<Task<bool>>(new Func<Task<bool>>(async() => { 

                bool isFinished = false;
                Device.BeginInvokeOnMainThread(async () =>
                {
                    await App.Current.MainPage.DisplayAlert("Message", msg, "Ok");
                    isFinished = true;
                });
                while (!isFinished) await Task.Delay(10);
                return true;
            })));
        }

C# Class "Promise" :

public class Promise<T>
    {
        Task<T> tache;
        public Promise(Task<T> tache)
        {
            this.tache = tache;
        }
        public Promise(Task<Task<T>> tache)
        {
            this.tache = tache.Result;

        }
        public void then(Action<T> onfinish)
        {
            this.tache.ContinueWith(rep =>
            {
                onfinish.Invoke(rep.Result);
            });
        }

    }

Javascript code :

var repAlert=alert("valeur interne = "+valeur);
    repAlert.then(function(rep){
        console.log("journal ok rep="+rep); // return Boolean value (true)
    });
    console.log("journal ok repAlert="+repAlert); // Return Promise object

The catch function is not implemented but you'll can do with this example...

This code is a part of a Xamarin project

ChrML commented 3 years ago

Bumping this one for a question: Since Jint is an interpreter, are there any reasons that C# methods cannot be async/return Task, but made look synchronously to the user-script?

I have a case with very many small simple scripts, that should call async methods. I'd like to avoid the complexity of introducing await/Promise objects to the user scripts, because none of the scripts have parallelism requiring actual async code.

ayende commented 3 years ago

You want to be able to do await, then? But do that automatically. One of the purposes of the Task API is to let you run things in the background, after all.

ChrML commented 3 years ago

You want to be able to do await, then? But do that automatically. One of the purposes of the Task API is to let you run things in the background, after all.

Yes, but the user scripts in my case don't really need it. Still, the host process needs it to avoid blocking the thread.

ayende commented 3 years ago

That might actually be a good idea to support, so the API isn't async based to the script, but is async for the usage.

lahma commented 3 years ago

Promise support has been implemented, but no support for async/await keywords yet.

John0King commented 2 years ago

May I ask what is block on this ? C# support callling any type that implment GetAwater() and result that implement ICriticalNotifyCompletion, and we can use TaskCompletionSource or IValueTaskSource to wrap any type/method that have callback action.

I wasn't see any problem from C# side (both js->c# and C# -> js)

ackava commented 2 years ago

@John0King Its been long since I answered, I have created my own JavaScript engine in C# which supports most of features of ES6 through ES2019. You can try and let me know.

lahma commented 1 year ago

Jint now has async/await support (Promise-wise). I'm not sure if it answers the need in this issue though? Anyone want to extend the support to interop side (task.GetAwaiter().GetResult() probably)? Or can we close this as "too generic"?

viceice commented 1 year ago

so Jint supports native async / await, but doesn't convert c# Task and others to Promise and vice versa yet?

lahma commented 1 year ago

Native for .NET doesn't exist yet, only the JS fullfill/reject chaining. Some brave soul could implement wrapping Task into Promise.

viceice commented 1 year ago

ok, will try to have a look at it, as that's now the missing blocker for async usage 😅

lahma commented 1 year ago

Probably something like adding task with GetAwaiter().GetResult() to event loop and resolving to its result.

christianrondeau commented 1 year ago

Pretty please don't do that :) The whole point of await is freeing the thread and allowing the use of synchronization contexts. Really the only desirable implementation will be painful but very valuable : async all the way (TM) meaning ExecuteAsync, AwaitAsync and friends. If you want I can provide more material to back this, we've been through this in the past too :)

lahma commented 1 year ago

@christianrondeau got your attention, now waiting for the PR 😉

viceice commented 1 year ago

but we need to make sure there are no real parallel tasks, as it's not expected from JavaScript. there should only be one thread at a time, otherwise it would make interoperability much more complicated.

lahma commented 1 year ago

I think that would mean adding the tasks to main event loop and blocking and processing with RunAvailableContinuations, even if that sounds horrible, it's JS after all.

christianrondeau commented 1 year ago

Haha yeah I'd love to, but I'm also struggling with time :) This being said, the reason I didn't even try is because I don't understand promises internally, and I know it'll be another huge PR with tons of breaking signature changes.

Or duplicate every involved method to avoid the (minimal) overhead of the Task.

JS can be single threaded, but the promise itself can definitely wait on a C# task that is bound to multiple async tasks (Task.WhenAll). What's important is that after the promise completes, control is given back to the synchronous single threaded javascript code (like node).

My idea that I didn't validate yet was to have a duplicated Execute and Evaluate methods as well as a duplicated RunAvailableContinuations (async and not async), and throw in the non async RunAvailableContinuations if a promise is bound to a Task that is not completed. Not sure if it's that simple in practice though.

I guess the very first question to ask is whether you want to offer only async execute and evaluate methods and pay the small overhead, or provide both signatures and potentially have duplicated methods and tests.

christianrondeau commented 1 year ago

I can definitely support @viceice and maybe even try a prototype branch, since I'm the one who opened the issue one year ago... And to be honest, I have to do some gymnastics to work around this so I'd definitely still make use of it. Can't promise anything though.

lahma commented 1 year ago

I do have interest in performance and solutions supporting it, but I'm afraid that the true async paths and dispatch to pool would lose the benefits just by the overhead of the interpreter.

Of course hard to measure when only sync path available (in Jint). Async shines in high concurrency but I'm unsure if Jint or any JS interpreting runtime will benefit of it.

christianrondeau commented 1 year ago

Actually the main value is to use IO ports instead of locking a thread in the ThreadPool (asp.net being the most obvious example). That was also the great selling point of NodeJS when it came out, not concurrency (well, concurrency of the server, not of the script itself).

It's also very true for Jint, as right now loading a file or accessing resources on a blob storage or any remote location (or use SQL etc) will lock the web thread and eventually kill your web server scalability if you use GetAwaiter as a workaround to provide synchronous methods to Jint. So implementing fake async using GetAwaiter would actually be a trap for novice and uninformed developers using Jint, who may discover they suddenly require more machines with idle CPUs to support the same load.

I hope this makes sense, I can try and provide some more thorough explanation and references if you're not convinced yet :)

lahma commented 1 year ago

I think a demo using Jint with different approaches would be great. That would clarify the benefits and probably show the benefits. I have no idea what percent of users jump to actual waiting I/O from engine.

christianrondeau commented 1 year ago

I'll try and do two things then (I knew this would come back and bite me!)

  1. I'll at least try to see if I can make an incomplete and poorly tested PR to help scope the actual work and changes involved, and;
  2. Make a case for the need to have a good implementation of async

What I cannot do is estimate how many people would benefit, but anyone who runs Jint in a web server and has any kind of file, http, tcp wait will greatly benefit under scale (not under low volume though).

lahma commented 1 year ago

Awesome! One thing to note is that we probably cannot have async/await as c# constructs at the heart of runtime (expression and statement evaluation), that would probably break both performance and ability have deeply nested calls (stack grows a lot with async state machines).

christianrondeau commented 1 year ago

In theory, I think we wouldn't need to have them deeply nested. The await of JavaScript is really just "pausing" execution until the promise has been resolved, we can await any async promises, and when they are resolved, continue.

That said, that "pause executing when encountering await and return" would be a significant step forward without doing any Task and C# await stuff (that's one way to break down this issue in smaller pieces). Something like:

// Setup a totally normal promise...
Action<JsValue> resolveFunc = null;
engine.SetValue('f', new Func<JsValue>(() => {
  var (promise, resolve, _) = engine.RegisterPromise();
  resolveFunc = resolve;
  return promise;
}));

// Then, we run code with await in it.
engine.Execute("let x = 1; x = await f(); throw new Error(x)");
Assert.That(engine.Wait, Is.True);
// This would actually run until the await statement and return immediately (not blocking).
// The stack and internal state would stay as-is, and Engine would have a flag Wait

// At this point, you cannot do engine.Evaluate("x") because Jint is in Wait state. You could throw or "queue" the additional scripts (throwing sounds sane to me)

// Technically, you could do C# await here but Jint doesn't care, it's just pending.
await Task.Delay(100);

// Then we can resume execution.
resolveFunc(2);
engine.RunAvailableContinuations();
Assert.That(engine.Wait, Is.False);
// This sets x = 2 and throws. Either resolveFunc  runs continuations, or RunAvailableContinuations continues until the next promise.

That wouldn't be the final behavior for users but it would be very very close. The next step would be to add an AsyncPromise, which is a normal promise but that contains a Task<JsValue> function instead of an Action<JsValue>. Then, an eventual EvaluateAsync would simply run the code, and as long as the result is Wait, await the current Promise sequentially until none are left, and the final completion value is returned. But behind the scene, it would use exactly the same behavior as described above.

That first step would be deep in the bowels of Jint and way out of my league though ;)

lahma commented 1 year ago

Also pinging @twop to this discussion as he originally wrote the Promise implementation and is painfully aware about the event loop and task internals 😉

lahma commented 1 year ago

Also linking earlier discussion here: #1081

christianrondeau commented 1 year ago

Thanks for the link, I can see I'm not the only one who thought of this ;)

But I just thought of another MUCH simpler approach... I'll try and confirm if it could work but I don't see why not.

  1. Replace the foreach in JintStatementList by an IEnumerator (while(statements.MoveNext());)
  2. Set that enumerator into Engine as an internal property
  3. When hitting an await statement in the enumerator function...
    1. If the result is a resolved promise, just continue
    2. If the result is an unresolved promise, do context.Engine.Waiting = theAwaitableObject, and stop enumerating
  4. The current implementation of  RunAvailableContinuations would simply check whether theAwaitableObject has been resolved, and if so, simply resume the enumerator

And that's it. If I didn't miss something, this would rid of all the complexity surrounding the event loop. After all, .NET already has a lot of the mechanisms we need for this.

Then, adding actual async to this becomes quite simple, we only need to have a Promise implementation that contains the Task object and await it.

Do you see something wrong with this? I'll try and see if it's as simple as that in practice.

christianrondeau commented 1 year ago

Wait a second, you already implemented some of it as part of #1323 - that'll make things easier :)

lahma commented 1 year ago

When touching JintStatementList my main concern is performance. Might be a good idea to first try changing to enumerator (and other required constructs) inside of it and then check what for example DromaeoBenchmark reports after that. It's a very hot code path. There was a painful optimization spree recently too where stack size was tuned - there's a test case for that too ShouldAllowReasonableCallStackDepth.

Maybe also worth a look how other engines handle such things, like Nill.JS.

Wait a second, you already implemented some of it as part of https://github.com/sebastienros/jint/pull/1323 - that'll make things easier :)

Oh did I 😆

christianrondeau commented 1 year ago

Soooo I made a quick and dirty test, and changed the implementation of JintStatementList to something like this:

                using var enumerator = EnumerateStatements(context).GetEnumerator();
                while (enumerator.MoveNext())
                {
                    c = enumerator.Current;
                    if (c.Type != CompletionType.Normal)
                    {
                        return new Completion(c.Type, c.Value, c._source);
                    }
                    lastValue = c.Value;
                }

and

        private IEnumerable<Completion> EnumerateStatements(EvaluationContext context)
        {
            foreach (var pair in _jintStatements!)
            {
                var s = pair.Statement;
                var c = pair.Value.GetValueOrDefault();
                yield return s.Execute(context);
            }
        }

So, nothing fancy but this should capture the state of the executing code and therefore have most of the memory impact.

With simple foreach

| Method | Prepared |             FileName |       Mean |      Error |      StdDev |     Median |       Gen0 |       Gen1 |       Gen2 |     Allocated |
|------- |--------- |--------------------- |-----------:|-----------:|------------:|-----------:|-----------:|-----------:|-----------:|--------------:|
|    Run |    False |      dromaeo-3d-cube |  37.707 ms |  0.1477 ms |   0.1382 ms |  37.690 ms |  1071.4286 |   214.2857 |          - |    6736.09 KB |
|    Run |    False |    dromaeo-core-eval |   8.631 ms |  0.0276 ms |   0.0258 ms |   8.632 ms |    62.5000 |    15.6250 |          - |     321.64 KB |
|    Run |    False | dromaeo-object-array |  94.079 ms |  0.3010 ms |   0.2668 ms |  94.056 ms | 21833.3333 |  1333.3333 |   166.6667 |  101909.16 KB |
|    Run |    False | droma(...)egexp [21] | 527.562 ms |  2.6257 ms |   2.4561 ms | 528.352 ms | 21000.0000 |  8000.0000 |  5000.0000 |  179734.59 KB |
|    Run |    False | droma(...)tring [21] | 689.923 ms | 36.5190 ms | 107.6771 ms | 714.966 ms | 52000.0000 | 23000.0000 | 20000.0000 | 1326278.65 KB |
|    Run |    False | droma(...)ase64 [21] |  91.320 ms |  0.4886 ms |   0.4570 ms |  91.336 ms |  1500.0000 |   166.6667 |          - |    7658.71 KB |
|    Run |     True |      dromaeo-3d-cube |  36.319 ms |  0.1105 ms |   0.0980 ms |  36.326 ms |  1357.1429 |   214.2857 |          - |     6441.6 KB |
|    Run |     True |    dromaeo-core-eval |   8.715 ms |  0.0204 ms |   0.0190 ms |   8.716 ms |    62.5000 |          - |          - |        309 KB |
|    Run |     True | dromaeo-object-array |  93.430 ms |  0.3545 ms |   0.3316 ms |  93.404 ms | 21833.3333 |  1333.3333 |   166.6667 |     101870 KB |
|    Run |     True | droma(...)egexp [21] | 310.598 ms |  2.1691 ms |   2.0290 ms | 310.822 ms | 20000.0000 |  7000.0000 |  4000.0000 |  175229.21 KB |
|    Run |     True | droma(...)tring [21] | 658.091 ms | 50.1232 ms | 147.7893 ms | 724.897 ms | 61000.0000 | 32000.0000 | 29000.0000 | 1326124.82 KB |
|    Run |     True | droma(...)ase64 [21] |  91.512 ms |  0.5171 ms |   0.4837 ms |  91.476 ms |  1500.0000 |   500.0000 |          - |    7569.46 KB |

With IEnumerator

| Method | Prepared |             FileName |      Mean |     Error |     StdDev |    Median |       Gen0 |       Gen1 |       Gen2 |  Allocated |
|------- |--------- |--------------------- |----------:|----------:|-----------:|----------:|-----------:|-----------:|-----------:|-----------:|
|    Run |    False |      dromaeo-3d-cube |  39.96 ms |  0.245 ms |   0.229 ms |  40.01 ms |  1538.4615 |   230.7692 |          - |    8.65 MB |
|    Run |    False |    dromaeo-core-eval |  10.23 ms |  0.061 ms |   0.057 ms |  10.23 ms |   656.2500 |   203.1250 |          - |       3 MB |
|    Run |    False | dromaeo-object-array |  96.60 ms |  0.459 ms |   0.429 ms |  96.60 ms | 22000.0000 |  1333.3333 |   333.3333 |  100.81 MB |
|    Run |    False | droma(...)egexp [21] | 318.88 ms |  1.924 ms |   1.800 ms | 318.78 ms | 21000.0000 |  7000.0000 |  4000.0000 |  180.29 MB |
|    Run |    False | droma(...)tring [21] | 570.04 ms | 40.221 ms | 118.591 ms | 569.87 ms | 52000.0000 | 22000.0000 | 19000.0000 | 1298.68 MB |
|    Run |    False | droma(...)ase64 [21] |  96.51 ms |  0.398 ms |   0.372 ms |  96.56 ms |  2333.3333 |   500.0000 |          - |   11.15 MB |
|    Run |     True |      dromaeo-3d-cube |  38.73 ms |  0.111 ms |   0.098 ms |  38.77 ms |  1846.1538 |   384.6154 |          - |    8.36 MB |
|    Run |     True |    dromaeo-core-eval |  10.30 ms |  0.025 ms |   0.021 ms |  10.30 ms |   656.2500 |   156.2500 |          - |    2.99 MB |
|    Run |     True | dromaeo-object-array |  94.29 ms |  0.279 ms |   0.261 ms |  94.34 ms | 22000.0000 |  1333.3333 |   333.3333 |  100.77 MB |
|    Run |     True | droma(...)egexp [21] | 310.69 ms |  1.196 ms |   1.060 ms | 310.66 ms | 22000.0000 |  9000.0000 |  5000.0000 |   181.1 MB |
|    Run |     True | droma(...)tring [21] | 650.93 ms | 51.067 ms | 150.572 ms | 725.84 ms | 52000.0000 | 22000.0000 | 19000.0000 | 1298.66 MB |
|    Run |     True | droma(...)ase64 [21] |  99.81 ms |  0.271 ms |   0.240 ms |  99.82 ms |  2400.0000 |   400.0000 |          - |   11.06 MB |

But I think the IEnumerator idea still makes sense, but by implementing it manually in every JintStatement class would have pretty much zero impact, and might be necessary anyway.

The problem is that JintStatementList is called from multiple, potentially deep places, and we need to halt execution in the middle of something. So everything that can run a JintStatementList needs to also pause and return back to the Engine.

So, here's what I'm going to try next:

  1. On every JintStatement, instead of Completion Execute(EvaluationContext) I'll have bool MoveNext() (which does the actual execution) and Completion Current {get;} (which returns whatever was the last completion).
  2. Test for perf again, though that part does not worry me at all. It's just going to be a big commit.
  3. Everywhere we run a JintStatementList, after calling MoveNext, check if we should wait (await + unresolved promise or parent of) and if so, return, only keeping the index of the statements array.

I'm still unclear as to what the event loop implementation does though. Maybe it'll be useless after this or maybe I'm completely off track. I'll report back.

lahma commented 1 year ago

I think you are nearing the generators problem zone (unfinished PR). Statement list needs to be able to pause and snapshot state to continue from when called next time. Generally "resume from index 3".

christianrondeau commented 1 year ago

Aaah so you're saying that instead of actually pausing the JintStatementList and resuming it, that would be breaking the whole parsed script in chunks up front, each chunk being a piece of code that can run until an await statement? I hope that made sense, I'm not aware of how generators work but I didn't think it would span across functions. Do you believe I should stop what I'm currently doing?

For example, you should be able to do things like:

let x = 0;
function fn1() {
  await something;
  x++;
}
await fn1();
if(x != 1) throw;

So the await can actually return control back to C# and resume later.

Oh and by the way, this is mean:

let fn;
fn = function(t) {
  if(t > 0) {
    await fn(t--);
  }
}
await fn(5);

Right now, the JintFunctionDefinition is reused in recursive calls, which complicates things a little (e.g. it broke the Prism test). Instead of having a state directly in JintStatementList, I'll need to create a JintStatementListInvocation (or something) so multiple instances of it can keep the state. But otherwise, I think it'll work (unless you tell me to drop it) but it will (again) involve quite a lot of code.

For now I'll try and see how far I can go with it and reach out when I have an actual working example of what I'm going for.

christianrondeau commented 1 year ago

So, less words, more code. Here's the idea (just a small piece to make sure I'm not going into a wall). The JintStatementList.Execute function uses an enumerator instead of directly executing the code.

The idea from this point is to replace the JintStatementList.Execute (which enumerates) by JintStatementList.GetEnumerator (Which would return the enumerator): https://github.com/christianrondeau/jint/blob/gh-514-async/Jint/Runtime/Interpreter/JintStatementList.cs#L78

Everything that calls Execute would need to instead get the enumerator and loop through it, returning when it hits an await operation. You can see how it affects quite a lot of code, however most classes would simply return the Completion as-is (e.g. a function call returns a promise, we don't care if it's resolved or not, etc.) so I don't think it's that bad either.

So, what do you think? Is that worth pursuing?

christianrondeau commented 1 year ago

To keep you in the loop.

For the sake of the prototype, I'll instead make a SuspendPromise that inherits from JsValue but that code will be thrown away.

christianrondeau commented 1 year ago

All right I'll have to stop for a while. Instead of returning a weird SuspendPromise, I set engine._suspend = true; engine._suspendValue = promise and return null. That makes sense, however this means EVERY operation that can be followed by await (const x = await something, for(x = await something; x < await... you get the point) needs to check whether the operation has to be halted, and allow resuming.

https://github.com/christianrondeau/jint/blob/gh-514-async/Jint/Runtime/Interpreter/Expressions/JintAwaitExpression.cs#L42

NiL.JS have an elegant solution to this, they wrap (at parse time I think) all those operations in a SuspendableExpression, which takes care of checking whether post-execute, the engine is suspended. However most of Jint expressions would require the ability to deal with resuming anyway, unless we decompose things even more than they currently are.

I'm out for now :) I'll probably continue investigating this sometime in the future though.

lahma commented 1 year ago

I'm trying to answer/comment at least some bits, please re-ask the parts I'm missing 😉 I'll be mixing generators concepts here too so be aware...

Right now, the JintFunctionDefinition is reused in recursive calls, which complicates things a little (e.g. it broke the Prism test). Instead of having a state directly in JintStatementList, I'll need to create a JintStatementListInvocation (or something) so multiple instances of it can keep the state. But otherwise, I think it'll work (unless you tell me to drop it) but it will (again) involve quite a lot of code.

I was thinking for generators case to use same construct as Nill.JS has, basically a Dictionary<object, object> in engine that would allow script part (like JintStatementList) contain state and restore when needed via engine._suspendData[this].

Generators need this in all funky places, like in try-catch-finally, it's ok to even yield from finally block.

So, less words, more code. Here's the idea (just a small piece to make sure I'm not going into a wall). The JintStatementList.Execute function uses an enumerator instead of directly executing the code.

So far your JintStatementListEnumerator looks like the old for-loop, just with the state. Could the suspendData dictionary usage bring the needed functionality to existing solution without sacrificing performance? So generators could share extend the State class instance tailored for this.

I've been reading tc39.es/ecma262/#await and really wondering. It says "suspend" and "resume" as if it was magic; I understand from that that while the running evaluation context is determined, how you actually pause execution is up to the implementer. In that case, I think the approach makes sense; resuming would also restore the corresponding evaluation context, but I think that's an easy one.

Currently the suspend for await is just letting the engine run it's continuoations as then we should expect all promises to be settled. await is kind of a thread yield I guess...

JintAwaitExpression.EvaluateInternal returns a JsValue (object but it's force-casted in JintExpression.GetValue later). Shouldn't this return a Completion instead? I can work around that but I'm wondering if that's how it should be or if it's something that should be changed (and potentially depended on).

I think there shouldn't be need for completion type for expression. For yield we will need to signal suspend, but ECMA spec doesn't have suitable return type in CompletionType, at least when I tried in generators branch - so the suspendData + some flag is needed.

christianrondeau/jint@gh-514-async/Jint/Runtime/Interpreter/Expressions/JintAwaitExpression.cs#L42

This is now "competing" a bit with UnwrapIfPromise? I think people need public API to get value out from engine (maybe running the continuoations too). But the change here looks good with extra state checks etc. Conceptually this is a bit hard for me to understand as await should always block until resolve. So the return null seems a bit wrong.

EDIT

Right now, the JintFunctionDefinition is reused in recursive calls, which complicates things a little (e.g. it broke the Prism test). Instead of having a state directly in JintStatementList, I'll need to create a JintStatementListInvocation (or something) so multiple instances of it can keep the state. But otherwise, I think it'll work (unless you tell me to drop it) but it will (again) involve quite a lot of code.

That's a good point. Each invocation has its own execution context so that might help with this, not sure how though. JintFunctionDefinition should only contain static data so if there's per-invocation race it has wrong state persisted.

christianrondeau commented 1 year ago

Thanks for taking the time. You can ignore piggybacking on Completion or JsValue, that's clearly wrong (it was more of a way to make a cheap and fast proof of concept).

In the end, the challenge I see is mostly suspending execution. For example, here: https://github.com/sebastienros/jint/blob/main/Jint/Runtime/Interpreter/Expressions/JintUnaryExpression.cs#L70 you'd need to wait for the value before continuing. You have to options. Either you store the execution context with the closure, save the state, and write code to resume it later, OR you take advantage of .NET doing this through Task and async, but you have to pay a small memory and perf cost (maybe acceptable, we'd have to try).

Going the "async all the way" approach (which is what I'd strongly recommend in almost every project using any kind of file or network IO) would solve all problems at once, really. The only "but" is the overhead, and it's significant enough to consider in cases of 5 + 5 versus (await 5) + (await 5) which are both valid. This is what I'd try first, but it's also a breaking change since now Engine functions must be async.

Going the "suspend" approach means every expression must have a state (or instantiate something that holds the state, like I did for the statements list). Whether you use an enumerator-like approach or a state dictionary (enumerators would be better IMHO since you avoid having to clean up state and you don't have to do a hash check at all) works anyway.

If I got this right, "generators" are similar to C#'s "yield" statements on IEnumerable functions; however this is limited to function invocation, it won't actually suspend execution, right? In other words, should I hold for some additional work on Jint or is it worth it to continue exploring? As usual, I want to help, not generate additional noise :)

ChrML commented 1 year ago

Great discussion. I have a bit of input:

Please don't go for a synchronous approach (something like GetAwaiter().GetResult). In fact it's better to not support await at all than such approach, or atleast throw a NotSupportedException indicating that ExexuteAsync must be used to execute this expression.

Tasks don't imply concurrency. You can use tasks in a singlethreaded environment. The purpose of a task is to yield the thread for something else to do useful work while the script is waiting for some event to occur (such as a network response), so that the script can continue in the same singlethreaded context (but doesn't need to be the same thread) when completed.

My usage of JINT is running a bunch of small user scripts (1000's). If we support async and the user wants to e.g. call await API.Delay(30000); to wait 30 seconds before doing something, a synchronous approach would starve the application by needing to spawn 1000 threads just idling around doing nothing. When in fact 1 thread could do all the work, and other useful stuff while the scripts are just waiting.

NET has a very powerful system already for handling and queuing tasks. By utilizing it, in practice, a few threads could run 1000 separate JINT scripts, and not neccessarily restricting one thread to one JINT engine. The script could start on one thread, await some network IO, and then continue to run on another thread. Still singlethreaded from the engine/script's point of view because there is no concurrency.

christianrondeau commented 1 year ago

Agreed @ChrML. To be clear on the "suspend" approach, it doesn't suspend the thread, it just stops evaluating where it is and returns to the caller. Meaning you could do:

// something is a promise mapped to a C# async function
engine.Evaluate("await something; dosomething();");
await stuffThatResolvesSomethingAsync();
engine.Resume(); // run continuations

So, no thread locking and no actual Task/async/await support in Jint but the ability to suspend would already allow async, despite not in a nice and user friendly way.

Right now to work around this, I'm doing (simplified):

const tasks = engine.Evaluate("user code returning loading commands...")
await actualLoadingOfStuff(tasks);
engine.Evaluate("other user code acting on the loaded stuff...")

It complicates things, but it works. I do think that many people won't hit high server concurrency nor be under the risk of running malicious user scripts, but I still wouldn't make thread locking a default behavior.

Now the challenge is to make it work ;)

ChrML commented 1 year ago

@christianrondeau With such approach, one could implement a more user-friendly ExecuteAsync with this pseudo-code:

public Task<JsValue> ExecuteAsync(Script script, CancellationToken ct)
{
    JsValue result = this.Execute(script);
    while (this.HasAsyncContinuation)
    {
        ct.ThrowIfCancellationRequested();
        await this.CallCurrentAwaitedTask(ct);  // Pass ct automatically to the user's async method if it's the last argument for cancellation.
        result = this.Resume();
    }
    return result;
}
christianrondeau commented 1 year ago

That's exactly it, yes; this (the suspend approach) has the advantage of not introducing Tasks everywhere in the code (async all the way), but it does increase complexity since you need every expression to be able to suspend and resume (unless there's some generator magic I'm missing ;) )

ChrML commented 1 year ago

That's exactly it, yes; this (the suspend approach) has the advantage of not introducing Tasks everywhere in the code (async all the way), but it does increase complexity since you need every expression to be able to suspend and resume (unless there's some generator magic I'm missing ;) )

I suspect the suspend logic will already be needed to implement JS generators, to some extent. So this could be an extension of that.

I agree that tasks all the way would be unneccessary, and probably make the performance lower for scripts that don't need it. So I'm all for the suspend approach.

ackava commented 1 year ago

I went through lot of answers and couldn't help putting few points, I hope it helps.

  1. Async/Await is a simple wrapper over generator, check how typescript transpiles async/await to ES6 code, await is replaced with yield. https://www.typescriptlang.org/play?target=2#code/G4QwTgBA7g5gpgFwgXgiAzgTwHYGMIAUArmADYCUKAfBAN4BQETEuA9tukmHOimlCACWSAGaJcAC2JlyAbkbM2HJAjgAPJKhADhEbugB0qjQTkKmS9K1JwDpVjALGEZgL5A , in this example, engine needs only a generator.
  2. Generator is different from the IEnumerable in C# as generator can receive the input parameter, so the next awaitable promise can be passed to generator to form a promise chain.
  3. There is no async JSValue, or no AsyncPromise, basically await can be used with any object that has then method. Take a look at the following code.
    var a = async () => {
    return await {
        then(n) {
            console.log("Then called");
            n("result");
        }
    };
    }
    a().then((r) => console.log(r));
  4. Eval must return a simple JSValue, which will contain then, catch and finally methods. It will be caller's responsibility to hook then to create Task. as shown below in an extension method..
    Task<JSValue> ToTask(this JSValue @this) {
        var then = @this["then"];
        if (!then?.IsFunction) {
             return Task.FromResult(@this);
        }
        var tcs = new TaskCompletionSource();
        then.InvokeFunction(
             new JSFunction( ...  => tcs.TrySetResult(arg0) ),
             new JSFunction( ...  => tcs.TrySetException(arg0.ToException()) );
        return tcs.Task;
    }
  5. Generator can be implemented in two ways (running generator on separate thread, and transpiling code to a state machine), I have tried both, first one is quick and easy, but very slow in performance.
  6. Separate thread generator is very easy to implement, pause current thread and run generator on a different thread till it reaches return/fails or it reaches yield. Current thread can continue when generator stops. All the states are easily maintained with only one issue that generator function does not run on main thread, causing problem for UI applications. And performance is slow for deeply nested generators.
  7. Transpiling into a state machine is complex to write but has excellent performance. First step is to change all loops and nested scopes into a simple flat scope, so you can put simple jumps between the statements. Because jumping in and out of nested scopes are not possible. Implementing TypeScript's generator transformation is quite complex.
  8. In case of interpretation, if we can create a concept of virtual thread, implementing generator could be simple. Real threads are very expensive.
christianrondeau commented 1 year ago

Thanks, in my personal case, anything that helps me better understand is welcome! I'm not clear on 2 however, I tried to re-read about generators and it looks like C#'s yield return statements. Maybe I should just wait for @lahma to write the generators ;)

I'd argue against using a thread though (I feel like everyone's agreeing already), since it would be worse than GetAwaiter(), since we'd use two threads now instead!

Transpiling in a flat scope sounds like the best option, though I'm not in a position to comment since I'm not familiar of what this actually involves. But it would make pausing and resuming straightforward.

For now, I still feel (emphasis on me not feeling comfortable enough to back this up with verifiable claims) that implementing the enumerable pattern on expressions to track state, and allowing to pause the engine and resume it within each expression would have both a low performance cost and wouldn't require too much refactoring; I also think that waiting to see how generators will be implemented would be wiser than trying to short-circuit that discussion. So for now, I'll watch but will be available-ish if you need more info on my PoC branch's approach (I'll leave it as-is for now).

EnCey commented 1 year ago

What I cannot do is estimate how many people would benefit, but anyone who runs Jint in a web server and has any kind of file, http, tcp wait will greatly benefit under scale (not under low volume though).

Yes, here! Just to add another vote to please not use GetAwaiter().GetResult() by default, we're building a simple workflow engine for our service based on Jint and async I/O is essential – pretty much like what @ChrML described already.


In case it is relevant to this discussion as an example use case, my approach to supporting async C# code with Jint promises is to capture all C# Tasks in a companion object to the Engine (AsyncContext) so that they can be awaited.

Basically there's an ExecuteAsync method that does this: (prototype code)

public async Task ExecuteAsync() {
  _engine.Execute("...");
  await asyncCtx.WhenAllPromisesAreSettled().ConfigureAwait(false);
}

From what I gathered in the discussion, this is similar to what @christianrondeau describes on a lower level, i.e. storing any "contiuations" that were created by a script and then executing them.

From my point of view, ideally Jint would provide an async overload of Execute/Evaluate for users who want to use async/Tasks ("async all the way" style) and the existing methods simply throw if a Task was captured.