r-lib / later

Schedule an R function or formula to run after a specified period of time.
https://r-lib.github.io/later
Other
137 stars 27 forks source link

Signal exhaustion of top-level callbacks with a user prompt #134

Open lionel- opened 4 years ago

lionel- commented 4 years ago

Code running at top-level might cause new output:

later::later(~ message("foo"), 1)
> foo

Or:

later::later(~ stop("foo"), 1)
> later: exception occurred while executing callback:
Evaluation error: foo.

When that is the case the REPL appears to be hanging because there is no trailing prompt after the output. This gives users the impression that R is busy.

This also causes issues in ESS where the only means of communication with R is trough the process output. If later causes new output at top-level without issuing a prompt, ESS will incorrectly consider R to be busy. Among other problems this stalls the evaluation queue.

Other handlers for running code outside the REPL such as options(error = ) or addTaskCallback() do not have this problem.

To fix this, perhaps later could output the prompt once the last callback has finished running:

cat(paste0("\n", getOption("prompt")))

This would signal users and ESS that R is ready to receive new commands.

wch commented 4 years ago

I think it could make sense to do this when at least one callback is run when the console is idle (that is, not from an explicit call to run_now().

However, some possible issues/problems with this approach:

lionel- commented 4 years ago

I think it could make sense to do this when at least one callback is run when the console is idle (that is, not from an explicit call to run_now().

That makes sense.

What happens if there's an active sink()?

Good point. Ideally the prompt would be echoed to stdout not to the sink, just like it happens when the user types commands. Can we can temporarily disable sinks?

Should the cat() also run if it's just a C callback (not R) that is run?

I think so. The C callback might also cause output or an error. The prompt is also a visible indicator that something happened.

wch commented 4 years ago

I just realized a problem with doing this. Most callbacks have no output, and if R is running in the terminal or in RStudio, then it will print a new prompt each time that happens. That will cause the console to scroll, and it will seem spontaneous from the user's perspective.

Is there any other way to signal to ESS that it's ready? If not, could you get changes into ESS to make that possible?

lionel- commented 4 years ago

Is there any other way to signal to ESS that it's ready? If not, could you get changes into ESS to make that possible?

Since ESS communicates with R exclusively via stdin and stdout I can't think of another way of signalling readiness. Displaying a prompt on exhaustion could be a global option that ESS would set though.

if R is running in the terminal or in RStudio, then it will print a new prompt each time that happens

It would print a new prompt in ESS as well. I was thinking this would be the desired behaviour. Even for me as a user it confuses me to see output without a prompt in the console because it makes it seem like R is executing something.

I assumed that exhaustion of later callbacks are a rare event but maybe it isn't? I could see how it would be problematic if it is frequent.

This might get complicated but maybe another better approach would be to sink output and only issue a prompt (and the output) when there was actually output. In this case the prompt could be displayed for each noisy callback rather than at exhaustion of the EL. I think that would be even better behaviour actually.

wch commented 4 years ago

I assumed that exhaustion of later callbacks are a rare event but maybe it isn't? I could see how it would be problematic if it is frequent.

For example, in the case of a httpuv web application running in the background, it could be serving many requests every second.

Displaying a prompt on exhaustion could be a global option that ESS would set though.

One concern that I have about sinking output or checking options -- or calling any R code at all -- is the overhead. The callback queue can potentially be flushed hundreds of times per second when the console is idle (with run_now(), it can be even more, I think). If these functions are called every time callbacks are executed, this could lead to significant overhead. I think it might make sense to only do this stuff if an option is set, and do some sort of throttling for sinking and checking options, so that the R code for that would only be called at most tens of times per second.