detecting an empty vat promise queue (end of "crank")

[dckc]

This test seems to be failing on xs (now that my tape work-alike is actually working):

https://github.com/Agoric/SwingSet/blob/7cd651107a31ba84d9b9f11610d72b5477035fd9/test/test-queue-priority.js#L5

{"actual":[1,2,3,5,6,4],"expected":[1,2,3,4,5,6]}

What's the justification for that test? Is the queue priority specified?

In what way does SwingSet rely on the queue priority?

cc @warner @phoddie

[dtribble]

JavaScript specifies that the promise queue is higher priority than the timer queue. As a result, JS is guaranteed that it will not take outside events until the promise queue is empty.

[phoddie]

@dtribble, you note:

JavaScript specifies that the promise queue is higher priority than the timer queue

Since timers are not defined by JavaScript, I assume you are referring more generally to events delivered by the host using a mechanism other than promises (e.g. timer callbacks). Some non-definitive web searching appears to confirm that this may be the case in browsers. However, I was unable to find text in ECMA-262 that defines this behavior. That said ECMA-262 is huge, so I may well have overlooked that. This morning I only focused on Jobs and Job Queues section.

If you wouldn't mind, would you help me with the normative reference for this behavior? Thank you!

[erights]

Attn @littledan What's the status of this?

[littledan]

@erights I think this PR is ready to land, but there may be an editorial alternative from @jmdyck coming soon.

[erights]

Link to PR please?

[littledan]

tc39/ecma262#1597

[erights]

Yes, that's what I was looking for. Thanks.

[dckc]

@littledan writes Sep 19:

We already have consensus on this layering change at TC39 ...

@phoddie that puts the ball in your court, yes? Would it help for me to open an issue in the Moddable repo?

Has anyone added test262 tests for this? At what point in the process does that usually happen? In a few recent cases I was a little concerned that fixes went into the Moddable repo without corresponding tests, but then it occurred to me that the tests should probably go into test262.

[phoddie]

When the specification in ECMA-262 settles down, we can look at revised behavior. Making the above example work as expected is straightforward. However, I expect there are more subtle behaviors implied by the changes under discussion. Good coverage in test262 would help make the intentions clear.

Finding an efficient implementation for what I understand the behavior to be looks to be non-trivial on microcontrollers where watch-dogs trigger if a single turn of the event loop runs for very long.

[erights]

What happens when these watch-dogs bark? Where can I read about this? I worry about the same consistency issue I explain at https://github.com/Agoric/proposal-oom-fails-fast

[phoddie]

Ha. Sorry for not explaining more. Let me try.

The watchdogs detect software hangs - infinite loops, deadlocks, etc. When they trigger, the usual action is to reboot the microcontroller. These are used to ensure an embedded device doesn't become unresponsive as a result of a bug. The threshold can be quite low - 100ms. There is considerable variation in the details across microcontrollers, and often there are many configuration options. The ESP32 watchdog is reasonably representative.

Unlike memory exhaustion, there's no consistency issue, as the watchdog reboots the entire device.

My concern is implementing the behavior under consideration for run loops where a promise resolves a promise that resolves another promise in a long chain. Each step may be short enough to avoid the watchdog, but in total they are not. To avoid that, it appears necessary to exit the run loop before servicing the all promises and resume on the next turn without allow timers/etc to execute. That's possible.

An easier work around is to "feed" the watchdog on each promise, effectively tell it "we didn't hang, we're just busy, so reset your timeout". But, there are problems with that. If the RTOS has work it only does when the event loop returns control to it, that would be deferred indefinitely. On an ESP8266, for example, the network stack runs at that time. Also, in my experience, feeding the watchdog has a bad habit of introducing subtle bugs that causing the watchdog to then fail to detect real issues.

Anyway... I don't think there's an issue here from a JavaScript or SES perspective. I just believe there are implementation concerns that will take some care to arrive at a result that is both correct and efficient on the embedded device XS targets.

[littledan]

Sorry, to clarify, the layering change does not say that Promises are higher priority than other jobs. That continues to be defined by the host environment. IMO it would be nice for the ecosystem to converge on this kind of scheduling where possible, but I understand if that's not appropriate for all environments.

[phoddie]

Thanks, @littledan. We're holding off on any implementation until the dust settles. To be clear, I'm not suggesting that we can't support the various event loop rules being discussed, only that there are some interesting challenges to implement them well in our target environments.

[dckc]

preempting queued callbacks looks tricky with glib

I found that the xs lin platform uses the glib main loop API. At first, this made me think addressing this issue is a matter of specifying G_PRIORITY_HIGH when queuing promise jobs.

But g_main_context_iteration () works like this:

Runs a single iteration for the given main loop. This involves checking to see if any event sources are ready to be processed, then if no events sources are ready and may_block is TRUE, waiting for a source to become ready, then dispatching the highest priority events sources that are ready.

Recall the test code:

  const log = [];
  setImmediate(() => log.push(1));
  setImmediate(() => {
    log.push(2);
    Promise.resolve().then(() => log.push(4));
    log.push(3);
  });
  setImmediate(() => log.push(5));
  setImmediate(() => log.push(6));

  t.deepEqual(log, [1, 2, 3, 4, 5, 6]);

By the time the push(4) is queued, glib has already prepared to dispatch 5 and 6 and using G_PRIORITY_HIGH doesn't preempt that.

I was going to say this can't be done with glib, but they do break down g_main_context_iteration in such a way that parts of it can be replaced:

Customizing the main loop iteration
Single iterations of a GMainContext can be run with g_main_context_iteration(). In some cases, more detailed control of exactly how the details of the main loop work is desired, for instance, when integrating the GMainLoop with an external main loop. In such cases, you can call the component functions of g_main_context_iteration() directly. These functions are g_main_context_prepare(), g_main_context_query(), g_main_context_check() and g_main_context_dispatch().

So we could make a customized version of g_main_context_dispatch that handles this sort of preemption. But it's around 100 lines of pretty tricky looking code.

[michaelfig]

Can we do something like:

const waitForPromises = inXS
  ? cb => setTimeout(cb, 1) // or other XS primitive?
  : cb => setImmediate(cb);

because AFAICT, setImmediate is only used by SwingSet to detect when no more promises will run. Or maybe some other primitive to execute only when the promise queue is done, not necessarily futzing with the relative priorities of the existing methods?

[dckc]

yes, @michaelfig , something like that. @warner and I chatted about this and now I think I understand the requirements better. It need not involve setTimeout at all. I updated the issue title as such.

[phoddie]

This seems complicated. Are you trying to implement the proposed behavior where settled promises always take priority over host callbacks? I believe you are working on Linux? If so, it would seem easier to modify your Linux host in lin_xs.c. The promises callbacks are invoked when fxQueuePromiseJobsCallback calls fxRunPromiseJobs. fxRunPromiseJobs calls all promises settled when it begins execution, but not those settled during its execution. Maybe something like this will do what you want? gboolean fxQueuePromiseJobsCallback(void *it) { txMachine* the = it; do { fxRunPromiseJobs(the); } while (mxPendingJobs.value.reference->next); return G_SOURCE_REMOVE; }

…

On Dec 6, 2019, at 5:19 PM, Dan Connolly ***@***.***> wrote: yes, @michaelfig <https://github.com/michaelfig> , something like that. @warner <https://github.com/warner> and I chatted about this and now I think I understand the requirements better. It need not involve setTimeout at all. I updated the issue title as such. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#45?email_source=notifications&email_token=ABUJ6CQT7WCLF3PUX6IFDO3QXL22NA5CNFSM4JTNXNW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGF2DQY#issuecomment-562799043>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABUJ6CSKGYQK43XCK2XJF6LQXL22NANCNFSM4JTNXNWQ>.

[dckc]

Thanks, @phoddie . Something like that may very well work.

But the core of the issue is not really priority of promises vs. host callbacks. It's about detecting when the promise queue of a "vat" under the control of a "kernel" is empty. The "kernel" was using setTimeout() to queue something that would only come back after the "vat" was done with all the promises it wanted to queue for itself.

[phoddie]

But the core of the issue is not really priority of promises vs. host callbacks. It's about detecting when the promise queue of a "vat" under the control of a "kernel" is empty. The "kernel" was using setTimeout() to queue something that would only come back after the "vat" was done with all the promises it wanted to queue for itself.

How do you do that in a browser?

…

On Dec 6, 2019, at 6:18 PM, Dan Connolly ***@***.***> wrote: Thanks, @phoddie <https://github.com/phoddie> . Something like that may very well work. But the core of the issue is not really priority of promises vs. host callbacks. It's about detecting when the promise queue of a "vat" under the control of a "kernel" is empty. The "kernel" was using setTimeout() to queue something that would only come back after the "vat" was done with all the promises it wanted to queue for itself. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#45?email_source=notifications&email_token=ABUJ6CVU4SEAFKTVSZQDXXDQXMBYBA5CNFSM4JTNXNW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGF3OLY#issuecomment-562804527>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABUJ6CSYIM2L6PXDOKD7YR3QXMBYBANCNFSM4JTNXNWQ>.

[dckc]

Browsers presumably work like node. The original test passes in chrome (with const setImmediate = f => setTimeout(f, 0)).

[phoddie]

I guess I'm not following the goals here well enough. I more-or-less understand how JavaScript behaves and how the XS host provides parts of that. I don't know about vats and kernels and the needs they have from promises. I apologize for jumping in. I'll stand aside on this one.

…

On Dec 6, 2019, at 9:20 PM, Dan Connolly ***@***.***> wrote: Browsers presumably work like node. The original test passes in chrome (with const setImmediate = f => setTimeout(f, 0)). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#45?email_source=notifications&email_token=ABUJ6CUTWJHS7WFRTY3A5PTQXMXCJA5CNFSM4JTNXNW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGF6GXA#issuecomment-562815836>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABUJ6CS4NTQ35QC2WO65HX3QXMXCJANCNFSM4JTNXNWQ>.

[dckc]

Yeah; I put "vat" and "kernel" in quotes because they're somewhat local terms of art.

There are some docs in https://github.com/Agoric/agoric-sdk/tree/master/packages/SwingSet/docs but I'm not sure even those are up to date.

No need to apologize; I think you may well have helped. But standing aside is fine too.

[dckc]

On xs, I'm getting:

# Exception: replay: cannot coerce undefined to object!

I traced it to empty playbackSyscalls at s = playbackSyscalls.shift():

agoric-sdk/packages/SwingSet/src/kernel/vatManager.js

Lines 214 to 216 in f833610

	function replay(name, ...args) {
	const s = playbackSyscalls.shift();
	if (djson.stringify(s.d) !== djson.stringify([name, ...args])) {

Is it possible this queue priority stuff is to blame?

[dckc]

fixed in #2194

[dckc]

double-check that the setImmediate queue is empty too in xsnap
(from discussion with @warner)

[dckc]

Oh yeah... we turned off setImmediate in the current xsnap. I wonder if/how liveslots is going to work without it. I'll press on and see...

[dckc]

In #2225 (comment) @kriskowal writes:

I remain weary that waitUntilQuiescent rests on the same slippery slope of ever deepening nested loops and priority queues as Node.js and the web. I fear that this creates a cottage industry for people who bear the deep knowledge of the hierarchy of micro-tasks and promise resolution, IO callbacks, timers, ticks, setImmediate, mutation observers, message passing, and now the notion of quiescence.

One can legitimately lose their mind sorting them out https://github.com/kriskowal/asap