Skip to content
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

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

Open
lionel- opened this issue Sep 8, 2020 · 5 comments
Open

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

lionel- opened this issue Sep 8, 2020 · 5 comments

Comments

@lionel-
Copy link
Member

lionel- commented Sep 8, 2020

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
Copy link
Member

wch commented Sep 9, 2020

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:

  • What happens if there's an active sink()?
  • I think that cat-ing the prompt would cause weird output in RStudio.
  • Should the cat() also run if it's just a C callback (not R) that is run?

@lionel-
Copy link
Member Author

lionel- commented Sep 10, 2020

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
Copy link
Member

wch commented Sep 10, 2020

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-
Copy link
Member Author

lionel- commented Sep 10, 2020

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
Copy link
Member

wch commented Sep 10, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants