1610682 - Eager evaluations doesn't recover from infinite loops

Status: RESOLVED FIXED

(DevTools :: Console, defect, P2)

Description

[:Harald Kirschner :digitarald]

Attachment: image.png

STR: With eager eval on type while (true); in Console

ER: Eager eval fails to run the loop and recovers gracefully.

AR: Browser locks up and slow-script dialog shows.

Comment 1

[:Harald Kirschner :digitarald]

Logan, is it possible to guard against long-running scripts in the eager eval backend?

Comment 2

[:Harald Kirschner :digitarald]

What would also help here and might be useful for general performance – if a new eager eval request it could cancel the currently running one.

Comment 3

[Logan Smyth [:loganfsmyth]]

Would it make sense to put a timeout on all eager evaluation? I can't really see a case where we'd want any eager eval to be long-running. If it's that long, we'd probably want to just wait for the user to finish typing what they were typing anyway. Even if we just said all eager eval requests have to run in 20ms or less or something. I think that would be easier than trying to cancel an in-progress operation.

That said, it does surprise me that the slow-script dialog doesn't come up eventually, so I'm also curious why that's happening. I'll leave myself NIed for now to try to look into that.

Comment 4

[:Harald Kirschner :digitarald]

Even if we just said all eager eval requests have to run in 20ms or less or something. I think that would be easier than trying to cancel an in-progress operation

I'd be in favor of that. It also caps overhead caused by eager eval.

That said, it does surprise me that the slow-script dialog doesn't come up eventually, so I'm also curious why that's happening. I'll leave myself NIed for now to try to look into that.

The dialog does come up, but I wasn't sure if it actually recovers. I think a few slow scripts queued up, so it's hard to pause effectively but it should work.

Comment 5

[Logan Smyth [:loganfsmyth]]

The dialog does come up, but I wasn't sure if it actually recovers.

Ahh gotcha. I'll have to check whether the console waits to dispatch an eager eval if the previous one is still running. If it doesn't, that's probably also a bug that we should file so that we don't accumulate a queue of things to eager-eval after we've stopped caring about the result.

Comment 6

[:Harald Kirschner :digitarald]

Logan, to make the situation better for the initial release in 74; would you propose a short-term fix or is capping eager eval execution time something we could aim to include for 74? This assumes there is no better long-term fix, which is my understanding from the comments so far.

Comment 7

[Logan Smyth [:loganfsmyth]]

I can look into it, but honestly I don't know, most likely that would be a bit rushed at this point. I know that SpiderMonkey has some infrastructure for interrupting execution, but I've never tried to use them and I don't know how easy it will be set up a timer to do what I want to do.

Comment 8

[Jason Laster [:jlast]]

Looked into this quickly and wrote a couple of helper functions fib(25) consistently takes ~70 ms, which would be a good candidate to be canceled:

• https://searchfox.org/mozilla-central/source/js/src/jsapi.cpp#4064-4066
• https://searchfox.org/mozilla-central/source/js/src/shell/js.cpp#4281-4285 (example usage)

function f(n) {
    if (n == 1 || n == 0) {
      return 1;
    }

    return f(n-1) + f(n-2)
}

function fib(m) {
  function f(n) {
    if (n == 1 || n == 0) {
      return 1;
    }

    return f(n-1) + f(n-2)
  }

  const startTime = new Date();   
  const r = f(m)
  const endTime = new Date(); 
  return [(endTime - startTime), r]
}

Comment 9

[Logan Smyth [:loganfsmyth]]

Attachment: Bug 1610682 - Time out of eager evaluation after 100ms. r=jlast!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ac19d46c04f9
Time out of eager evaluation after 100ms. r=jlast

Comment 11

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/ac19d46c04f9