scratchfoundation / scratch-vm

Virtual Machine used to represent, run, and maintain the state of programs for Scratch 3.0
http://scratchfoundation.github.io/scratch-vm/
GNU Affero General Public License v3.0
1.22k stars 1.54k forks source link

Promises always yield #688

Open cwillisf opened 7 years ago

cwillisf commented 7 years ago

Expected Behavior

It should be possible to run a particular opcode many times per tick as long as the implementation function runs quickly.

Actual Behavior

If the opcode implementation returns a promise, it can only run once per tick.

Steps to Reproduce

  1. Load an extension which runs in a worker
  2. Find a block in that extension which runs quickly and doesn't impact any visual elements on the stage
  3. Try to run that block in a loop
  4. Note that the loop runs slowly (once per frame)
cwillisf commented 7 years ago

Possibly related: #658

tmickel commented 7 years ago

Hmm, interesting. As far as I know, Promises only resolve when the JS event loop is idle, right? So in order to accomplish something like the above, the execution JS (i.e. interpreter) would have to yield the event loop. How do you know whether to yield the event loop or to continue executing other threads? Maybe like... anytime there is a Promise returned, drop out of execution and do a setTimeout(0) to re-enter, thereafter proceeding as usual with the rest of the Scratch tick? In that case though, any Promise that didn't resolve properly immediately would have the same problem. Guess you could periodically drop out and re-enter to allow Promise resolutions to interleave, and then do something mad cool with the thread ordering to bring the Promise-resolved-thread back to life in the same tick. Hmmmmm.... sounds hard but should be interesting :)

cwillisf commented 7 years ago

I believe resolving a promise happens synchronously, whenever the promise's resolve or reject function is called. I believe messages coming from workers will only arrive during the event phase (does anyone know the proper name for this?), which could cause similar behavior for worker-based extensions. We could probably optimize that such that only reporters exhibit this behavior, and if that's not enough explore some cache-based approaches.

Anyway, in pseudo-JS, I think we might be able to do something like this:

Promise.all(
  threads.map(
    thread => runNextBlock(thread)
  )
).then(() => {
  if (still time) {
    run_another_tick();
  }
}

Using Promise.race() with a timer might be useful here, too.

tmickel commented 7 years ago

hmm :) i should dig further into exactly what happens when a promise resolves. I'm not sure what you mean by they resolve synchronously but I'm not confident either.

Does this match your expectation?

new Promise(
  (resolve) => {
    console.log('message 0');
    resolve();
  }
).then(() => {
  console.log('message 1');
});
console.log('message 2');

Output: message 0 message 2 message 1

p.s. hope things are going well!!!!!! not trying to cause problems here just to save you time if possible ;p

cwillisf commented 7 years ago

Interesting - that's not the result I expected. Hmmmmmm...

paulkaplan commented 7 years ago

Is this the same issue as this one? https://github.com/LLK/scratch-vm/issues/760

griffpatch commented 7 years ago

I'm not sure... although I did wonder about that... but do broadcast and waits actual use promises? I couldn't find anywhere obvious that linked into the promises from the broadcast and wait code...

On 6 November 2017 at 14:27, Paul Kaplan notifications@github.com wrote:

Is this the same issue as this one?

760 https://github.com/LLK/scratch-vm/issues/760

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/LLK/scratch-vm/issues/688#issuecomment-342164339, or mute the thread https://github.com/notifications/unsubscribe-auth/AGbNvg0AvXBVw6EE6xDFps13uRZoWlhEks5szxdmgaJpZM4PuX38 .