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

Use threaded version of websocket #16

Closed
wants to merge 1 commit into from
Closed

Conversation

wch
Copy link
Collaborator

@wch wch commented Jan 6, 2020

This should be merged after rstudio/websocket#62.

Before this change and the corresponding one in websocket, Chromote idles at ~6.5% CPU usage. After these changes, it idles at 0%.

Can be tested with:

remotes::install_github('rstudio/websocket@wch-threads')
remotes::install_github('rstudio/chromote@threaded-websocket', dependencies = FALSE)

@wch wch force-pushed the threaded-websocket branch from e9d7433 to 926564c Compare January 6, 2020 22:57
@wch
Copy link
Collaborator Author

wch commented Jan 8, 2020

Now that I've thought about this a bit more, I think we can't remove the polling code just yet. It would require r-lib/later#99 to be resolved before we could make this change.

This PR will work when Chromote is used in synchronous mode, but when in async mode, it won't work at all, because nothing will be pumping the child event loop.

For example:

library(chromote)
b <- ChromoteSession$new()

# Synchronous mode
b$Runtime$evaluate("1+1") %>% str()
# List of 1
#  $ result:List of 3
#   ..$ type       : chr "number"
#   ..$ value      : int 2
#   ..$ description: chr "2"

In async mode, this results in no output:

b$Runtime$evaluate("1+1", wait_ = F)$
  then(function(value) {
    str(value)
  })

But if you run the private child event loop a few times, it will print the value:

for (i in 1:10) {
  later::run_now(0.2, loop = b$get_child_loop())
}
# List of 1
#  $ result:List of 3
#   ..$ type       : chr "number"
#   ..$ value      : int 2
#   ..$ description: chr "2"

@wch
Copy link
Collaborator Author

wch commented Feb 7, 2020

Closing in favor of #24.

@wch wch closed this Feb 7, 2020
@wch wch deleted the threaded-websocket branch February 7, 2020 19:10
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

Successfully merging this pull request may close these issues.

1 participant