-
-
Notifications
You must be signed in to change notification settings - Fork 563
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
WIP: Scheduler #1082
Open
SebastianStehle
wants to merge
6
commits into
sebastienros:main
Choose a base branch
from
SebastianStehle:scheduler
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
WIP: Scheduler #1082
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
f3eb195
Scheduler.
SebastianStehle 9c67783
Another test
SebastianStehle ce7ed7c
Get rid of EvaluateAsync.
SebastianStehle 8a4daf7
Fix tests.
SebastianStehle e252098
Fix scheduler usage.
SebastianStehle ac93f8b
Fix scheduler.
SebastianStehle File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
using System; | ||
using System.Threading.Tasks; | ||
using Xunit; | ||
|
||
namespace Jint.Tests.Runtime | ||
{ | ||
public class SchedulingTests | ||
{ | ||
class Context | ||
{ | ||
public string Result { get; set; } | ||
} | ||
|
||
[Fact] | ||
public async Task CanRunAsyncCode() | ||
{ | ||
var engine = new Engine(); | ||
|
||
var ctx = new Context(); | ||
|
||
engine.SetValue("ctx", ctx); | ||
engine.SetValue("setTimeout", new Action<Action, int>(async (callback, delay) => | ||
{ | ||
using (var task = engine.CreateTask()) | ||
{ | ||
await Task.Delay(delay); | ||
|
||
task.Invoke(callback); | ||
} | ||
})); | ||
|
||
await engine.ExecuteAsync(@" | ||
setTimeout(function () { | ||
ctx.Result = 'Hello World'; | ||
}, 100); | ||
"); | ||
|
||
Assert.Equal("Hello World", ctx.Result); | ||
} | ||
[Fact] | ||
public async Task CanRunNestedAsyncCode() | ||
{ | ||
var engine = new Engine(); | ||
|
||
var ctx = new Context(); | ||
|
||
engine.SetValue("ctx", ctx); | ||
engine.SetValue("setTimeout", new Action<Action, int>(async (callback, delay) => | ||
{ | ||
using (var task = engine.CreateTask()) | ||
{ | ||
await Task.Delay(delay); | ||
|
||
task.Invoke(callback); | ||
} | ||
})); | ||
|
||
await engine.ExecuteAsync(@" | ||
setTimeout(function () { | ||
setTimeout(function () { | ||
setTimeout(function () { | ||
ctx.Result = 'Hello World'; | ||
}, 100); | ||
}, 100); | ||
}, 100); | ||
"); | ||
|
||
Assert.Equal("Hello World", ctx.Result); | ||
} | ||
|
||
[Fact] | ||
public async Task CanRunSyncCode() | ||
{ | ||
var engine = new Engine(); | ||
|
||
var ctx = new Context(); | ||
|
||
engine.SetValue("ctx", ctx); | ||
engine.SetValue("setTimeout", new Action<Action, int>((callback, delay) => | ||
{ | ||
using (var task = engine.CreateTask()) | ||
{ | ||
task.Invoke(callback); | ||
} | ||
})); | ||
|
||
await engine.ExecuteAsync(@" | ||
setTimeout(function () { | ||
ctx.Result = 'Hello World'; | ||
}, 100); | ||
"); | ||
|
||
Assert.Equal("Hello World", ctx.Result); | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
using System; | ||
|
||
namespace Jint.Scheduling | ||
{ | ||
internal class DeferredTask : IDeferredTask | ||
{ | ||
private readonly Scheduler _scheduler; | ||
private bool _isCompleted; | ||
|
||
public DeferredTask(Scheduler scheduler) | ||
{ | ||
_scheduler = scheduler; | ||
} | ||
|
||
public void Dispose() | ||
{ | ||
Cancel(); | ||
} | ||
|
||
public void Cancel() | ||
{ | ||
if (_isCompleted) | ||
{ | ||
return; | ||
} | ||
|
||
_isCompleted = true; | ||
|
||
_scheduler.Cancel(this); | ||
} | ||
|
||
public void Invoke(Action action) | ||
{ | ||
if (_isCompleted) | ||
{ | ||
return; | ||
} | ||
|
||
_isCompleted = true; | ||
|
||
_scheduler.Invoke(this, action); | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
using System; | ||
|
||
namespace Jint.Scheduling | ||
{ | ||
public interface IDeferredTask : IDisposable | ||
{ | ||
void Invoke(Action action); | ||
|
||
void Cancel(); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Threading.Tasks; | ||
|
||
namespace Jint.Scheduling | ||
{ | ||
internal sealed class Scheduler : IDisposable | ||
{ | ||
private readonly TaskCompletionSource<object> _taskCompletionSource = new TaskCompletionSource<object>(); | ||
private readonly HashSet<DeferredTask> _pendingTasks = new HashSet<DeferredTask>(); | ||
private readonly Queue<Action> _inlinedTasks = new Queue<Action>(); | ||
private bool _isRunning; | ||
private bool _isDisposed; | ||
|
||
public Task Completion | ||
{ | ||
get => _taskCompletionSource.Task; | ||
} | ||
|
||
public void Dispose() | ||
{ | ||
_isDisposed = true; | ||
} | ||
|
||
public IDeferredTask CreateTask() | ||
{ | ||
var task = new DeferredTask(this); | ||
|
||
_pendingTasks.Add(task); | ||
|
||
return task; | ||
} | ||
|
||
public void Invoke(DeferredTask task, Action action) | ||
{ | ||
if (_isDisposed) | ||
{ | ||
return; | ||
} | ||
|
||
_pendingTasks.Remove(task); | ||
|
||
if (_isRunning) | ||
{ | ||
_inlinedTasks.Enqueue(action); | ||
return; | ||
} | ||
|
||
_isRunning = true; | ||
try | ||
{ | ||
action(); | ||
|
||
RunAvailableContinuations(); | ||
|
||
TryComplete(); | ||
} | ||
catch (Exception ex) | ||
{ | ||
_taskCompletionSource.TrySetException(ex); | ||
} | ||
finally | ||
{ | ||
_isRunning = false; | ||
} | ||
} | ||
|
||
internal void Cancel(DeferredTask deferredTask) | ||
{ | ||
_pendingTasks.Remove(deferredTask); | ||
|
||
TryComplete(); | ||
} | ||
|
||
private void TryComplete() | ||
{ | ||
if (_pendingTasks.Count == 0) | ||
{ | ||
_taskCompletionSource.TrySetResult(true); | ||
} | ||
} | ||
|
||
private void RunAvailableContinuations() | ||
{ | ||
var queue = _inlinedTasks; | ||
|
||
while (true) | ||
{ | ||
if (queue.Count == 0) | ||
{ | ||
return; | ||
} | ||
|
||
var nextContinuation = queue.Dequeue(); | ||
|
||
// note that continuation can enqueue new events | ||
nextContinuation(); | ||
} | ||
} | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a strong opinion that Jint should not have
Task
as a part of it's API surface area, otherwise there is an implicit promise (no pun intended) of taking care of thread safety, which is not something Jint is interested in (I might be wrong here).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe as a small piece of "evidence", no JS engines I know provide comprehensive async support. You basically have to do all the steps I described in the issue in a host environment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean with async support?
For me, in this context "async" is everything that is not invoked immedately. SetTimeout, setInterval, Promises, callbacks and so on. Even module loading is actually a promise in javascript.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean the actual async runtime is usually outside of JS engines, e.g. they have a low level primitive that a hosting environment needs to implement. Specifically:
HostEnqueuePromiseJob
from the language spec https://www.ecma-international.org/wp-content/uploads/ECMA-262_12th_edition_june_2021.pdfThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's a bit easier link: https://tc39.es/ecma262/#sec-hostenqueuepromisejob , I haven't really gone into this but basically custom hosts should be possible in Jint (we should make it so and improve when features are missing), there's already the Host concept and ideally Jint would have all expected behavior and allow adding missing more controversial bits.